Skip to content

Commit 827c48f

Browse files
edenoclaude
andcommitted
Merge domain-boundary & ownership cleanup (phase-8-5-domain-boundaries)
Behavior-preserving architecture hardening before browser QA: app-wide day-validation + repair routing → src/domain/validation.js; shadow-export → src/domain/shadowExport.js; bad-channel converter semantics → src/domain/badChannels.js; override-cleanup decisions → src/domain/deviceOverrides.js; risky workspace transitions → src/state/workspaceTransitions.js. Added architecture guard tests (pages → domain/state only; no page→sibling-page domain import). Review-round changes (deliberate, tested; golden baselines stay byte-identical): config versions allocated by max(version)+1; atomic createConfigurationSnapshotAndApplyForward is the single reconfiguration action (orphaned addConfigurationSnapshot/applyConfigurationForward removed); repair UX (missing-maps → Channel Maps, Devices badge reflects day-owned errors, Validation/Export button dedup, honest override copy). Gate: full vitest (3904 pass), 125 golden baselines byte-identical, 0 lint errors, clean build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 parents ece5a27 + 1e692dc commit 827c48f

50 files changed

Lines changed: 3176 additions & 1808 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/docs/plans/pre-cutover-export-correctness/PLAN.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
# Pre-cutover Export Correctness Implementation Plan
22

3-
**Status:** Phases 1–5 complete (export gate fails closed; device resolution / day bad-channel merge; Hardware Config wiring + identity safety; schema-valid device output — integer IDs end-to-end, required device fields, per-shank offsets, no stray keys, ntrode-/group-ID uniqueness; subject/session completeness + DANDI — weight/description collected, timestamp DOB, Latin-binomial species + no-slash ids, required `experiment_description`, and an in-Overview subject repair surface). Phases 6–11 not started in this main plan; Phase 8.5 has been added as the pre-QA domain-boundary hardening gate. Workflow discoverability/mental-model concerns are now recorded as an explicit Phase 11 input in [workflow-clarity-design.md](workflow-clarity-design.md). Note: an interim refinement made electrode-group `location` optional in the editor and then reverted to required-with-targeted-default per lab decision (see overview.md / REFACTOR_CHANGELOG).
3+
**Status:** Phases 1–5 are recorded as complete in this plan snapshot (export gate fails closed; device resolution / day bad-channel merge; Hardware Config wiring + identity safety; schema-valid device output — integer IDs end-to-end, required device fields, per-shank offsets, no stray keys, ntrode-/group-ID uniqueness; subject/session completeness + DANDI — weight/description collected, timestamp DOB, Latin-binomial species + no-slash ids, required `experiment_description`, and an in-Overview subject repair surface). Later phase implementation status should be verified from the current branch and changelog; Phase 8.5 is the pre-QA domain-boundary hardening gate and Phase 8.6 is the pre-QA workflow-clarity/setup-UX gate. Note: an interim refinement made electrode-group `location` optional in the editor and then reverted to required-with-targeted-default per lab decision (see overview.md / REFACTOR_CHANGELOG).
44

55
The new multi-page workspace UI can currently hand `trodes_to_nwb` a YAML file that is
66
silently wrong — missing electrode probes, missing day-level bad-channel marks, schema-invalid
77
device IDs, or a date-of-birth the schema rejects — with no error shown to the user. This plan
88
closes those export-correctness defects so the workspace path produces valid, complete metadata,
99
and hardens the surrounding import and persistence edges. **Scope note:** phases 1–8 are the
10-
export-correctness fixes; Phase 8.5 is behavior-preserving pre-QA architecture hardening; phases 9–11
10+
export-correctness fixes; Phase 8.5 is behavior-preserving pre-QA architecture hardening; Phase 8.6
11+
makes the user workflow and setup UX understandable before browser QA; phases 9–11
1112
extend the plan to pre-cutover **QA and UX readiness** (browser regression QA, a Claude-executable
1213
usability/behavior audit, and a professional-UX-polish audit) so the workspace is not only correct but
1314
coherent and safe for repeated scientific use. It is the correctness
@@ -42,6 +43,7 @@ For agent invocation, **load only the slice you need**:
4243
- [phase-7-import-persistence-hardening.md](phase-7-import-persistence-hardening.md) — **P2:** the shared nested-error-path fix (workspace gate messages) + the explicitly scoped legacy partial-import exception; guard the empty-blob load crash and the failed-autosave unsaved-work gap.
4344
- [phase-8-optogenetics-correctness.md](phase-8-optogenetics-correctness.md) — **P1 (opto sessions):** add/verify the workspace opto entry path, fix the trodes_to_nwb opto key mismatches (`optogenetic_stimulation_software`, `volume_in_uL`) with a schema-safe transition, and validate all-or-nothing completeness so an opto session isn't silently dropped.
4445
- [phase-8-5-domain-boundaries-ownership-cleanup.md](phase-8-5-domain-boundaries-ownership-cleanup.md) — **pre-QA architecture hardening:** move app-wide validation/repair routing and converter semantics out of page modules, extract the riskiest workspace transitions into pure helpers, and add architecture guard tests before Playwright verifies the integrated path.
45-
- [phase-9-playwright-qa-pass.md](phase-9-playwright-qa-pass.md) — **browser regression QA:** add a Playwright workspace regression suite and runbook that exercise the corrected Phases 1–8.5 flows in a real browser: mistake-prevention UX, repair navigation, export gating/preflight/download, persistence recovery, and opto on/off behavior.
46+
- [phase-8-6-workflow-clarity-setup-ux.md](phase-8-6-workflow-clarity-setup-ux.md) — **pre-QA workflow clarity:** add a domain-backed setup/readiness model and align Animal Workspace, Animal Editor, Day Devices, reconfiguration, Validation, and Export around the user's workflow so electrode setup and existing-data review are discoverable before Playwright QA.
47+
- [phase-9-playwright-qa-pass.md](phase-9-playwright-qa-pass.md) — **browser regression QA:** add a Playwright workspace regression suite and runbook that exercise the corrected Phases 1–8.6 flows in a real browser: mistake-prevention UX, repair navigation, export gating/preflight/download, persistence recovery, workflow clarity, and opto on/off behavior.
4648
- [phase-10-claude-usability-behavior-audit.md](phase-10-claude-usability-behavior-audit.md) — **Claude-executable usability/proper-behavior audit:** run scripted scenario matrices, mistake injection, UI/state/export triangulation, keyboard/viewport checks, and produce a findings/fix log before cutover.
4749
- [phase-11-professional-ux-polish-audit.md](phase-11-professional-ux-polish-audit.md) — **professional UX polish audit (not the v3 cutover phase):** run a Claude-executable workflow-clarity, design-system, interaction-consistency, accessibility, content, responsive-layout, and perceived-performance pass with fixes/findings.

‎.claude/docs/plans/pre-cutover-export-correctness/app-code-organization-review.md‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,73 @@ Plan disposition: the pre-QA subset of this review is now incorporated as
1010
Treat the phase file as the executable source of truth. This note remains the architectural rationale and
1111
should be refreshed at the start of Phase 8.5 before code changes begin.
1212

13+
## Refreshed inventory at Phase 8.5 start (verified against the branch)
14+
15+
The review below predates phases 1–8 in wording but is structurally accurate. Verified current state
16+
before extraction, with the exact symbols/targets each task moves:
17+
18+
- **App-wide validation/repair routing lives in `src/pages/DayEditor/validation.js`** (906 lines). It
19+
exports `validateDay`, `computeStepStatus`, `computeDevicesStatus`, `computeEpochsStatus`,
20+
`dayOverrideIssues`, `groupErrorsByStep`, `stepIdForIssue`, `repairTargetForIssue`,
21+
`animalEditorStepForFieldPath`, `STEP_LABELS`, `SURFACE_BY_CODE`, `ANIMAL_EDITOR_STEPS`, plus the
22+
page-only field-blur helper `validateField`. Phase 8 added opto/fs_gui routing here
23+
(`SURFACE_BY_CODE.partial_configuration/multiple_excitation_sources`, the `fs_gui` branches in
24+
`stepIdForIssue`/`deriveSurfaceFromPath`, the Optogenetics entry in `ANIMAL_EDITOR_STEPS`) — that
25+
routing moves intact.
26+
- **Consumers (verified):** same-folder `DayEditorStepper` (`computeStepStatus`), `ExportStep`
27+
(`computeStepStatus`, `validateDay`, `STEP_LABELS`), `OverviewStep` (`validateField` — page-only,
28+
stays), `RepairActions` (`repairTargetForIssue`, `STEP_LABELS`), `ValidationStep`
29+
(`groupErrorsByStep`, `validateDay`); and the two **cross-page (sibling) importers that are the
30+
real violations** — `pages/AnimalEditor/AnimalEditorStepper.jsx`
31+
(`animalEditorStepForFieldPath`) and `pages/ValidationSummary/index.jsx` (`computeStepStatus`).
32+
- **Task 1 target:** move everything except `validateField` to `src/domain/validation.js`; repoint
33+
all consumers (including Day Editor's own) at the domain module; leave `validateField` in
34+
`pages/DayEditor/validation.js`.
35+
- **Bad-channel + override converter semantics live in render bodies (Task 2 targets):**
36+
- `pages/DayEditor/BadChannelsEditor.jsx` — multi-shank detection (`getProbeShanks().length > 1 &&
37+
ntrodes.length > 1`), later-row mark translation (`translateLaterRowMarks`), the probe-wide
38+
migration union (`handleProbeWideToggle`), and invalid-mark detection.
39+
- `pages/AnimalEditor/ChannelMapEditor.jsx` — the same multi-shank/translation/migration logic plus
40+
`validateChannelMaps` (probe-local range, multi-shank first-row, scalar-tolerance).
41+
- `pages/DayEditor/DevicesStep.jsx` — `validateBadChannels` (multi-shank first-row range) and the
42+
malformed/stale/shadowing override-cleanup decisions (`wholeOverridesMalformed`,
43+
`badChannelContainerMalformed`, `staleOverrideKeys`, `corruptValueKeys`, `presentGeometryKeys`),
44+
which **mirror `dayOverrideIssues`**.
45+
- **Task 2 target:** pure `src/domain/badChannels.js` (multi-shank rule, later-row translation,
46+
probe-wide map build, invalid-mark interpretation, range check) and `src/domain/deviceOverrides.js`
47+
(`classifyDeviceOverrides`), with a test that the classifier corresponds 1:1 to `dayOverrideIssues`.
48+
- **At Phase 8.5 start, risky workspace transitions lived inline in `src/state/useWorkspace.js`
49+
(Task 3 targets):** the `updateAnimal` devices branch (mirrors the edit into the latest snapshot,
50+
lines ~228-248), the reconfiguration snapshot/apply actions, `rebuildConfigurationHistory`, `createDay`,
51+
and `updateDay`'s malformed-session guard. The completed Phase 8.5 cleanup moved the transition recipes
52+
into `src/state/workspaceTransitions.js`, made the atomic `createConfigurationSnapshotAndApplyForward`
53+
the only public reconfiguration action, removed the orphaned two-action store API, and pins new days to
54+
the latest snapshot's actual version rather than the history length. Electrode-group/channel-map mutation
55+
recipes still live in
56+
`pages/AnimalEditor/AnimalEditorStepper.jsx` (`handleSaveGroup`, `confirmDeleteGroup`,
57+
`handleCopyConfirm`, `handleSaveChannelMap`).
58+
- **Task 3 target:** pure `src/state/workspaceTransitions.js` with the four named transitions;
59+
`useWorkspace` keeps hydration, autosave, debounce, localStorage, the existence-check throws, and
60+
the `workspaceRef`/version-return orchestration.
61+
- **`src/state/repairCommands.js`** is the executable-repair executor (current command set:
62+
`resetDayCollection`, `resetAnimalCameras`, `resetDataAcqDevice`, `rebuildConfigurationHistory`,
63+
`resetDeviceOverrides`, `removeDeviceOverrideKey`, `resetBadChannelOverrides`,
64+
`removeBadChannelOverrideKey`, `resetDaySession`) — already a state-layer module; not moved.
65+
- **`pages/DayEditor/shadowExport.js`** is pure export-truth behavior (encoder-stability check) that a
66+
sibling page (`ValidationSummary`) imports cross-folder. **Task 1/4 target:** move to
67+
`src/domain/shadowExport.js` so no page imports export-truth from a sibling page.
68+
69+
### Deferred at Phase 8.5 (recorded, not expanded)
70+
71+
- **Issue #4 (legacy/workspace facade in `store.js`)** — legacy-path relocation/removal is explicitly
72+
out of scope for this phase (and gated on the v3 cutover). Not touched.
73+
- **Issue #5 (schema-aligned types)** — type generation/sync is deferred to post-cutover per the phase
74+
scope guard; only done if type drift directly blocks the phase (it does not).
75+
- **`pages/DayEditor/SaveIndicator.jsx`** is a shared **presentational** component imported by
76+
`AnimalEditor/HardwareConfigStep`. It is not app-wide domain behavior, so relocating it to
77+
`src/components` is out of this phase's scope; the architecture guard allowlists this single
78+
presentational cross-page import and forbids all others.
79+
1380
## Verdict
1481

1582
The app is moving in the right direction, but it is not yet cleanly organized. The recent

‎.claude/docs/plans/pre-cutover-export-correctness/designs.md‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,16 @@ fixed at record time — pinning matches the science.)
3333
surface. `updateAnimal({ devices })` writes **both** `animal.devices` **and**
3434
`configurationHistory[latest].devices`, keeping them in sync. *This is the new behavior that fixes the
3535
P0:* configuring probes now flows into version 1's snapshot, so days pinned to it export the probes.
36-
- **`createDay`** pins `day.configurationVersion = animal.configurationHistory.length` (the latest
37-
version) — already its behavior (`useWorkspace.js:410-433`); now that version actually holds the config.
36+
- **`createDay`** pins `day.configurationVersion` to the latest snapshot's actual version number (not the
37+
history length, so non-contiguous repaired histories still pin to a resolvable version). Now that latest
38+
version actually holds the config.
3839
- **Reconfiguration is fork-before-edit** (the explicit freeze the High-severity review finding asked
3940
for). The wizard's "Reconfigure from day X" action, run **before** the user edits the new geometry:
40-
1. `addConfigurationSnapshot(animal.id, { devices: clone(latest snapshot), … })` → a new version N+1
41-
that starts **identical** to the current config (returns N+1 — the v3-plan return-value contract).
42-
2. `applyConfigurationForward(animal.id, N+1, [day X … end])` → repoints the chosen days to N+1.
43-
3. Set `animal.devices` to mirror version N+1 (the new latest).
41+
1. `createConfigurationSnapshotAndApplyForward(animal.id, { devices: clone(latest snapshot), ... }, [day X ... end])`
42+
creates a new version N+1 that starts **identical** to the current config and repoints the chosen days
43+
to it in one atomic transition.
44+
2. `animal.devices` is already the mirror of that cloned latest config. When the user edits geometry after
45+
the fork, `updateAnimal({ devices })` writes the mirror and the new latest snapshot together.
4446

4547
Now days **before** X stay pinned to version N (its snapshot frozen with the *old* config), and days
4648
**X…end** are on N+1. When the user then edits geometry in the Animal Editor, `updateAnimal` writes
@@ -61,10 +63,11 @@ copy-on-write-per-keystroke and no draft layer — the fork is the single, expli
6163
The Phase-10.5 wizard currently reads already-edited `animal.devices` as `nextConfig` and renders a
6264
live-vs-snapshot diff. Under model B that diff is dropped (your call: "apply the current configuration to
6365
days X–Y" with no diff). The wizard becomes a **fork-point selector**: pick the boundary day + the day
64-
range, confirm, fork (steps 1–3 above); geometry is then edited in the Animal Editor against the new
65-
latest version. Keep the existing `addConfigurationSnapshot` / `applyConfigurationForward` actions and
66-
the returned-version contract; only the wizard's call order and its UI change. Keep
67-
`reconfigWorkflow.integration.test.js` / `ReconfigWizard.test.jsx` green, updated to the model.
66+
range, confirm, fork with the atomic action above; geometry is then edited in the Animal Editor against the new
67+
latest version. The public store entry point is `createConfigurationSnapshotAndApplyForward`; the old
68+
two-action store API (`addConfigurationSnapshot` then `applyConfigurationForward`) has been removed as
69+
production-dead. Keep the pure transition helpers and `reconfigWorkflow.integration.test.js` /
70+
`ReconfigWizard.test.jsx` green, updated to the atomic model.
6871

6972
### What phase 2 must prove
7073

0 commit comments

Comments
 (0)