feat: Chip-on-Board (CoB) packaging support (#259) - #265
Conversation
Design for issue #259: editable boolean Project.chip_on_board, a snapshot field on ManufacturabilityCheck, --cob precheck wiring (the image already supports it), and a service-layer re-check on toggle (TriggerReason.COB_CHANGE). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Spec reviewer flagged a false 'slot/DRC-version snapshot' precedent: do_starting reads slot_size/full_id live from check.project, not a snapshot. Drop the proposed ManufacturabilityCheck.chip_on_board snapshot field and read --cob live from check.project (consistent with --slot/--id); fix service class reference. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per review feedback, CoB toggling now mirrors the DRC_UPDATE/precheck-version mechanism instead of the file-replacement cancel path: a new model method create_check_cob_change() (parallel to create_check_drc_update) creates a new PENDING check with parent_check linkage and TriggerReason.COB_CHANGE; the existing _cancel_superseded_checks() auto-cancels any in-progress older check. Pure ORM, so no service/task-import layer is needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Spec review found _cancel_superseded_checks() is only called from checks_cleanup(), which is NOT in CELERY_BEAT_SCHEDULE, so the relied-upon auto-cancel never runs. DRC updates never hit this because they only re-check FINISHED checks. Since a CoB toggle can happen mid-check, create_check_cob_change() now marks an in-progress latest check as CANCELLING explicitly (mark_cancelling), relying only on the scheduled checks_pending/checks_cancelling pollers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Spec reviewer (approved) flagged that the existing create_check_drc_update docstring repeats the unscheduled-cleanup belief; capture as a follow-up so it is not copied into create_check_cob_change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Assuming PR #262 lands, checks_cleanup_superseded runs on a 60s beat and _cancel_superseded_checks/checks_cleanup no longer exist. Update the spec's rationale: the explicit cancel in create_check_cob_change() is kept for immediacy (a superseded check must not finish with a stale CoB flag), audit-precise cancel reason, and test determinism; the scheduled cleanup is demoted to a defence-in-depth backstop. Drop the now-obsolete docstring follow-up (fixed by #262) and note the rebase-after-#262 dependency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Spec review findings: checks_cancelling only transitions CANCELLING->CANCELLED; container removal is done by checks_cleanup_orphaned_docker (60s) once the check is CANCELLED — correct all four passages that attributed teardown to checks_cancelling. Recast the explicit-cancel justification in terms of legal state transitions (ANALYZING->FINISHED vs CANCELLING->CANCELLED only) instead of the imprecise 'active' invariant, and add the detail-page CoB badge to the test plan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
8 TDD tasks: model field + migration, COB_CHANGE trigger + migration, create_check_cob_change(), --cob command wiring, form + template, view re-check hook, detail badge, final verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Make Task 1's editability test fetch fresh from the DB so it genuinely fails before the field exists; give Task 5's form tests their own user/project (TestProjectForm.setUp only creates a shuttle) and correct both tasks' expected-failure descriptions; fix the create_check_drc_update end-line reference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add boolean USER_FIELD for Chip-on-Board packaging opt-in. Defaults to False, always user-editable (not subject to core-field immutability). Migration adds the field to both project and historicalproject tables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extends ManufacturabilityCheck.TriggerReason with COB_CHANGE ("cob_change")
to support re-triggering checks when a project's chip_on_board option changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task 2's quality review found that _file_display.html's trigger-reason badge chains have no else fallback (a COB_CHANGE check would render no badge) and manufacturability_check_status.html falls back to a generic label. Add cob_change branches to all four chains plus a check-history badge test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add ManufacturabilityCheck.create_check_cob_change() which creates a PENDING check with TriggerReason.COB_CHANGE, explicitly cancels any in-progress source check (unlike DRC update which relies on superseded- check cleanup), and guards against being called on a non-latest check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Append --cob to the Docker precheck command in do_starting when project.chip_on_board is True; add TDD test confirming the flag is present (cob=True) and absent (cob=False via existing regression test). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add chip_on_board field to ProjectForm.Meta.fields/widgets/help_texts and render it in project_form.html after is_public. The field is a user field (not a CORE_FIELD) so it stays enabled for non-staff on existing projects. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Quality review polish: drop the Meta.help_texts duplicate (ModelForm inherits the model field's help_text) and trim an overstated test docstring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add form_valid override on ProjectUpdateView that detects chip_on_board changes via form.changed_data and calls create_check_cob_change() on the latest check, cancelling any in-progress check and queuing a new PENDING COB_CHANGE check for the scheduled dispatcher to pick up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Quality review fixes: replace the len([check, 'new']) lint-evasion idiom with EXPECTED_CHECKS_AFTER_COB_TOGGLE in tests/constants.py, annotate the _make_submitted_check helper, and derive the form payload's name/description from setUp's project instead of duplicating the strings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add Chip-on-Board badge to project detail page and CoB Change trigger-reason badges to all four if/elif chains in _file_display.html and manufacturability_check_status.html. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Quality review polish: assert the unambiguous 'Chip-on-Board (CoB)' string in the positive test, and make the standard-packaging test assert the Packaging block and Standard badge actually render rather than only absence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThis PR implements Chip-on-Board (CoB) packaging support by adding an editable ChangesChip-on-Board Packaging Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
wafer_space/projects/tests/test_forms.py (1)
92-101: ⚡ Quick winUse factories instead of direct ORM object creation in tests.
Line [92] and Line [95] create test data with
User.objects.create_user(...)and
Project.objects.create(...). Please use factory-boy here to keep test setup
consistent with the suite’s test-data contract.♻️ Proposed refactor
+from wafer_space.projects.tests.factories import ProjectFactory +from wafer_space.users.tests.factories import UserFactory @@ - user = User.objects.create_user( - username="formuser", email="form@example.com", password=TEST_PASSWORD - ) - project = Project.objects.create( - user=user, - name="Form Project", - shuttle=self.shuttle, - project_id="FRMP", - ) + user = UserFactory(username="formuser", email="form@example.com") + project = ProjectFactory( + user=user, + name="Form Project", + shuttle=self.shuttle, + project_id="FRMP", + )As per coding guidelines, "Use factory-boy for creating test data, not pytest fixtures."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wafer_space/projects/tests/test_forms.py` around lines 92 - 101, Replace direct ORM creation in the test with factory usage: instead of User.objects.create_user(...) and Project.objects.create(...), use the project's factory classes (e.g., UserFactory and ProjectFactory) to create the user and project, passing the same attributes (username, email, password/TEST_PASSWORD via factory params, and shuttle and project_id to ProjectFactory) and then instantiate ProjectForm with the factory-created user and instance; update references to User, Project, ProjectForm, TEST_PASSWORD, UserFactory and ProjectFactory accordingly so test data follows the suite’s factory conventions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wafer_space/projects/models.py`:
- Around line 2330-2343: The create_check_cob_change method performs unlocked
reads and writes causing race conditions; fix it by wrapping the critical
section in a transaction.atomic block and reloading the key row with a
select_for_update lock (e.g., re-fetch self.project_file or the related
ManufacturabilityCheck via
project_file.latest_manufacturability_check.select_for_update()) before
re-evaluating latest_manufacturability_check and is_cancellable, then call
mark_cancelling and ManufacturabilityCheck.objects.create only after the locked
re-check to ensure a single authoritative decision and prevent duplicate
COB_CHANGE checks.
In `@wafer_space/projects/tests/test_views.py`:
- Around line 208-225: The test
test_detail_shows_cob_change_badge_in_check_history currently only creates one
ManufacturabilityCheck (via ManufacturabilityCheckFactory) so the COB_CHANGE
entry may be treated as the first entry and not rendered by the history branch
in _file_display.html; update the test to create an earlier (older) check for
the same project_file (e.g., a finished check with a different trigger_reason or
earlier timestamp) before creating the COB_CHANGE check so that the COB_CHANGE
check is a non-first history entry and the history-path rendering is exercised.
- Around line 499-512: The test test_unchanged_cob_creates_no_check currently
ignores the POST response so it can false-pass on form validation errors; after
calling self.client.post(url, self._cob_form_data(chip_on_board=False)) capture
the response, assert its status_code indicates success (e.g., 302 for redirect
after successful POST or 200 if no redirect), then refresh the related object
(use the project_file returned by self._make_submitted_check or reload it from
the DB) and assert its chip_on_board/project flag remains unchanged, and finally
assert
ManufacturabilityCheck.objects.filter(project_file=check.project_file).count()
== 1 to ensure no new check was created.
In `@wafer_space/projects/views.py`:
- Around line 257-261: After saving the project, calling
latest_check.create_check_cob_change() can raise ValueError and cause a 500;
wrap that call in a try/except that catches ValueError (only) and swallow or log
the error so the successful save isn't rolled back or bubbled as a 500. Locate
the call to latest_manufacturability_check.create_check_cob_change() in the view
(where cob_changed is handled), wrap it in try/except ValueError as e, and use
the view/logger to record a warning with the exception message while allowing
form_valid to return normally.
---
Nitpick comments:
In `@wafer_space/projects/tests/test_forms.py`:
- Around line 92-101: Replace direct ORM creation in the test with factory
usage: instead of User.objects.create_user(...) and Project.objects.create(...),
use the project's factory classes (e.g., UserFactory and ProjectFactory) to
create the user and project, passing the same attributes (username, email,
password/TEST_PASSWORD via factory params, and shuttle and project_id to
ProjectFactory) and then instantiate ProjectForm with the factory-created user
and instance; update references to User, Project, ProjectForm, TEST_PASSWORD,
UserFactory and ProjectFactory accordingly so test data follows the suite’s
factory conventions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e071124-a98f-4a6e-9533-1d2233cd0133
📒 Files selected for processing (17)
docs/superpowers/plans/2026-06-10-chip-on-board-packaging.mddocs/superpowers/specs/2026-06-08-chip-on-board-packaging-design.mdwafer_space/projects/forms.pywafer_space/projects/migrations/0056_historicalproject_chip_on_board_and_more.pywafer_space/projects/migrations/0057_alter_manufacturabilitycheck_trigger_reason.pywafer_space/projects/models.pywafer_space/projects/tasks_checks.pywafer_space/projects/tests/constants.pywafer_space/projects/tests/test_forms.pywafer_space/projects/tests/test_models.pywafer_space/projects/tests/test_tasks.pywafer_space/projects/tests/test_views.pywafer_space/projects/views.pywafer_space/templates/projects/_file_display.htmlwafer_space/templates/projects/manufacturability_check_status.htmlwafer_space/templates/projects/project_detail.htmlwafer_space/templates/projects/project_form.html
| def test_detail_shows_cob_change_badge_in_check_history(self): | ||
| """Detail page shows CoB Change badge for COB_CHANGE trigger in history.""" | ||
| project_file = ProjectFileFactory(project=self.project) | ||
| self.project.submitted_file = project_file | ||
| self.project.save() | ||
| ManufacturabilityCheckFactory( | ||
| project=self.project, | ||
| project_file=project_file, | ||
| status=ManufacturabilityCheck.Status.FINISHED, | ||
| trigger_reason=ManufacturabilityCheck.TriggerReason.COB_CHANGE, | ||
| ) | ||
| self.client.login(username="testuser", password=TEST_PASSWORD) | ||
| url = reverse("projects:detail", kwargs={"pk": self.project.pk}) | ||
|
|
||
| response = self.client.get(url) | ||
|
|
||
| assert response.status_code == HTTP_OK | ||
| assert "CoB Change" in response.content.decode() |
There was a problem hiding this comment.
test_detail_shows_cob_change_badge_in_check_history does not guarantee history-path coverage.
On Lines 213-218, only one COB_CHANGE check is created. In _file_display.html, history badges are rendered only for non-first entries, so this can pass without validating the history mapping branch. Add a newer check so COB_CHANGE is definitely rendered as history.
Suggested test-shape adjustment
ManufacturabilityCheckFactory(
project=self.project,
project_file=project_file,
status=ManufacturabilityCheck.Status.FINISHED,
trigger_reason=ManufacturabilityCheck.TriggerReason.COB_CHANGE,
)
+ # Create a newer/latest check so COB_CHANGE is shown in history, not current.
+ ManufacturabilityCheckFactory(
+ project=self.project,
+ project_file=project_file,
+ status=ManufacturabilityCheck.Status.FINISHED,
+ trigger_reason=ManufacturabilityCheck.TriggerReason.RETRY,
+ )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wafer_space/projects/tests/test_views.py` around lines 208 - 225, The test
test_detail_shows_cob_change_badge_in_check_history currently only creates one
ManufacturabilityCheck (via ManufacturabilityCheckFactory) so the COB_CHANGE
entry may be treated as the first entry and not rendered by the history branch
in _file_display.html; update the test to create an earlier (older) check for
the same project_file (e.g., a finished check with a different trigger_reason or
earlier timestamp) before creating the COB_CHANGE check so that the COB_CHANGE
check is a non-first history entry and the history-path rendering is exercised.
| def test_unchanged_cob_creates_no_check(self): | ||
| """Submitting the form with CoB unchanged creates no new check.""" | ||
| check = self._make_submitted_check(ManufacturabilityCheck.Status.FINISHED) | ||
| self.client.login(username="testuser", password=TEST_PASSWORD) | ||
| url = reverse("projects:update", kwargs={"pk": self.project.pk}) | ||
|
|
||
| self.client.post(url, self._cob_form_data(chip_on_board=False)) | ||
|
|
||
| assert ( | ||
| ManufacturabilityCheck.objects.filter( | ||
| project_file=check.project_file | ||
| ).count() | ||
| == 1 | ||
| ) |
There was a problem hiding this comment.
test_unchanged_cob_creates_no_check can false-pass when form POST is invalid.
On Line 505, the response is ignored; if validation fails, check count can stay 1 and the test still passes. Assert the response status and project flag to ensure the request succeeded and the “unchanged” path was actually exercised.
Suggested hardening
- self.client.post(url, self._cob_form_data(chip_on_board=False))
+ response = self.client.post(url, self._cob_form_data(chip_on_board=False))
+ assert response.status_code == HTTP_FOUND
+ self.project.refresh_from_db()
+ assert self.project.chip_on_board is False
assert (
ManufacturabilityCheck.objects.filter(
project_file=check.project_file
).count()
== 1
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_unchanged_cob_creates_no_check(self): | |
| """Submitting the form with CoB unchanged creates no new check.""" | |
| check = self._make_submitted_check(ManufacturabilityCheck.Status.FINISHED) | |
| self.client.login(username="testuser", password=TEST_PASSWORD) | |
| url = reverse("projects:update", kwargs={"pk": self.project.pk}) | |
| self.client.post(url, self._cob_form_data(chip_on_board=False)) | |
| assert ( | |
| ManufacturabilityCheck.objects.filter( | |
| project_file=check.project_file | |
| ).count() | |
| == 1 | |
| ) | |
| def test_unchanged_cob_creates_no_check(self): | |
| """Submitting the form with CoB unchanged creates no new check.""" | |
| check = self._make_submitted_check(ManufacturabilityCheck.Status.FINISHED) | |
| self.client.login(username="testuser", password=TEST_PASSWORD) | |
| url = reverse("projects:update", kwargs={"pk": self.project.pk}) | |
| response = self.client.post(url, self._cob_form_data(chip_on_board=False)) | |
| assert response.status_code == 302 | |
| self.project.refresh_from_db() | |
| assert self.project.chip_on_board is False | |
| assert ( | |
| ManufacturabilityCheck.objects.filter( | |
| project_file=check.project_file | |
| ).count() | |
| == 1 | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wafer_space/projects/tests/test_views.py` around lines 499 - 512, The test
test_unchanged_cob_creates_no_check currently ignores the POST response so it
can false-pass on form validation errors; after calling self.client.post(url,
self._cob_form_data(chip_on_board=False)) capture the response, assert its
status_code indicates success (e.g., 302 for redirect after successful POST or
200 if no redirect), then refresh the related object (use the project_file
returned by self._make_submitted_check or reload it from the DB) and assert its
chip_on_board/project flag remains unchanged, and finally assert
ManufacturabilityCheck.objects.filter(project_file=check.project_file).count()
== 1 to ensure no new check was created.
Addresses a CodeRabbit finding on PR #265: "latest" and is_cancellable were read off an unlocked in-memory instance, so a concurrent transition could (a) clobber a freshly FINISHED check back to CANCELLING from a stale in-memory status, or (b) let two simultaneous CoB toggles each create a COB_CHANGE check. Wrap the body in transaction.atomic() and re-read the row with select_for_update() before any decision, so the choice is made against committed state and concurrent toggles serialize on the row lock. The new test reproduces the FINISHED-clobber deterministically (no threads): it updates the row out-of-band, then asserts the locked re-read leaves the committed FINISHED status untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1FsYafhYvbwqWVCkGDPYJ
Addresses a CodeRabbit finding on PR #265: create_check_cob_change() can raise ValueError when the source check is no longer its file's latest. Because it runs after super().form_valid() has already saved the project, and ATOMIC_REQUESTS wraps the whole request in one transaction, the raise rolled the request back into a 500 and discarded the user's valid CoB edit. Catch the recoverable ValueError, log a warning, and let the request finish normally. The model's transaction.atomic() block means the inner savepoint rolls back cleanly, leaving the committed project save intact; the newer check that superseded this one picks up the new flag on its own. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1FsYafhYvbwqWVCkGDPYJ
The project detail page showed non-Chip-on-Board projects as "Standard" packaging. The correct term for an unpackaged die is "Bare Die", so rename the badge label and update the detail-view test accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1FsYafhYvbwqWVCkGDPYJ
Summary
Closes #259.
Lets users request Chip-on-Board (CoB) packaging for a project. When set, the
manufacturability precheck runs with the precheck image's
--cobflag (the realargparse flag — the issue originally said
--chip-on-board), enabling the extraCoB compatibility checks. Toggling the option re-runs the check.
Design spec:
docs/superpowers/specs/2026-06-08-chip-on-board-packaging-design.mdImplementation plan:
docs/superpowers/plans/2026-06-10-chip-on-board-packaging.mdWhat changed
Project.chip_on_board— editable boolean (a USER_FIELD, not core-immutable); migrations0056(+HistoricalProjectmirror) and0057(newTriggerReasonchoice).TriggerReason.COB_CHANGE— new trigger reason for re-checks caused by a toggle.ManufacturabilityCheck.create_check_cob_change()— modelled oncreate_check_drc_update(), but cancels an in-progress check itself viamark_cancelling(reason="Chip-on-Board option changed")before creating the new PENDING check. This guarantees a superseded check can never FINISH with a result computed from the old CoB setting (ANALYZING→FINISHED is legal; CANCELLING permits only →CANCELLED). Pure ORM — no new Celery tasks or schedules; the existingchecks_pending/checks_cancellingpollers do the async work.do_starting— appends--cobto the precheck Docker command whencheck.project.chip_on_boardis set, read live from the project exactly like--slot/--id.ProjectUpdateView.form_validcreates the re-check when the flag changes (draft projects just persist the flag); Packaging badge on the project detail page;cob_changebranches added to all four trigger-reason badge chains (_file_display.htmlpreviously rendered no badge for unknown reasons).Relationship to #261 / #262
The design assumed PR #262 (
checks_cleanup_superseded) would land first, but does not depend on it: the explicit cancel increate_check_cob_change()is the mechanism; the scheduled superseded-cleanup is only defence-in-depth once #262 merges.Known gap (deliberate, needs a follow-up issue)
Django admin can edit
chip_on_board(ProjectAdminexposes all fields) without triggering the cancel+recreate logic, so an admin toggle during an in-flight check leaves it running with the old setting. The spec scoped the toggle hook to the project edit view; recommend a fast-follow to add asave_modelhook (or make the field admin-readonly) and addchip_on_boardto adminlist_display/list_filter.Test plan
create_check_cob_changebehaviors incl. cancel-on-RUNNING and non-latest guard, exact--cobcommand position, form presence/editability, view toggle/draft/unchanged paths, detail + check-history badges)make check-allclean (ruff, mypy, djlint)makemigrations --check— no missing migrations🤖 Generated with Claude Code
Summary by CodeRabbit