Skip to content

Commit 7363f03

Browse files
committed
Merge ts-state-camerausage: type the day camera-usage helpers under strict TS
state/cameraUsage.js → strict .ts (behavior-preserving). Reads day collections through the typed selectors (getDayTasks/getDayAssociatedVideos/getDayFsGuiYamls) instead of inline Array.isArray guards (behavior-equivalent); resolveDayCameraUsage → Camera[]. Two small documented reads remain (FsGuiYaml interface gap on camera_id; refs.has(... as string)). Verified: typecheck clean, cameraUsage (20) + baselines (145) byte-identical, full suite 4793, CI=true build, e2e 104. Code-reviewer: behavior-preserving, zero findings — exported YAML unchanged for any input.
2 parents 6e891d4 + 5937f58 commit 7363f03

3 files changed

Lines changed: 55 additions & 33 deletions

File tree

‎.claude/docs/research/architecture-assessment.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ or enforced tokens** (F6), and **deferred lint debt that weakens the build gate*
3636

3737
| Gap | Evidence | Why it bites refactoring |
3838
| --- | --- | --- |
39-
| **No static types** ⏳ PARTIAL | Incremental TS underway (`tsc --noEmit` CI gate live): `io/yaml.ts`, `state/workspaceTypes.ts` + the task-catalog modules, `domain/deviceOverrideMerge.ts`, and the Phase-9a validation leaf modules (`repairRouting`/`geometryProvenance`/`dayOverrideValidation`/`stepStatus`) are `.ts` under `strict`; the **entire `validation/` directory** (leaf predicates + `rules/*` families + `rulesValidation` composer + `index`/`schemaValidation`/`rawShape`/`quickChecks`/`useQuickChecks` + the shared `issueTypes.ts`) is now `.ts` under `strict`; the `state/` migration is underway — the persistence layer (`state/workspaceMigrations.ts` + `state/persistence.ts`) the canonical read layer (`state/workspaceSelectors.ts`, `unknown`-in / typed-container-out), and a first selector-consumer (`state/configDiff.ts`) are `.ts`. Remaining: most of `state/` (`workspaceUtils`/`workspaceTransitions`/`workspaceActions`/…) + `domain/`, most components, `useWorkspace`, and the legacy form path stay JS (`checkJs:false`) | Rename a domain fn or change an `Animal`/`Day` shape → **silent until a test happens to exercise it.** No safe IDE rename, no null-safety. Tooling agent scored type-safety **2/10** (improving as the pure core converts). |
39+
| **No static types** ⏳ PARTIAL | Incremental TS underway (`tsc --noEmit` CI gate live): `io/yaml.ts`, `state/workspaceTypes.ts` + the task-catalog modules, `domain/deviceOverrideMerge.ts`, and the Phase-9a validation leaf modules (`repairRouting`/`geometryProvenance`/`dayOverrideValidation`/`stepStatus`) are `.ts` under `strict`; the **entire `validation/` directory** (leaf predicates + `rules/*` families + `rulesValidation` composer + `index`/`schemaValidation`/`rawShape`/`quickChecks`/`useQuickChecks` + the shared `issueTypes.ts`) is now `.ts` under `strict`; the `state/` migration is underway — the persistence layer (`state/workspaceMigrations.ts` + `state/persistence.ts`) the canonical read layer (`state/workspaceSelectors.ts`, `unknown`-in / typed-container-out), and the first selector-consumers (`state/configDiff.ts`, `state/cameraUsage.ts`) are `.ts`. Remaining: most of `state/` (`workspaceUtils`/`workspaceTransitions`/`workspaceActions`/…) + `domain/`, most components, `useWorkspace`, and the legacy form path stay JS (`checkJs:false`) | Rename a domain fn or change an `Animal`/`Day` shape → **silent until a test happens to exercise it.** No safe IDE rename, no null-safety. Tooling agent scored type-safety **2/10** (improving as the pure core converts). |
4040
| **Oversized files/components** ⏳ PARTIAL | ~~`domain/validation.js` 1142~~ ✅ split (Phase 9a) into routing/provenance/producers/composer/step-status modules · ~~`validation/rulesValidation.js` 1108~~ ✅ split into `validation/rules/*` families (reference / opto / channelMap / electrodeGroup / dandiSubject / identity / behavioralEvent) + a 96-LOC composer · `valueList.js` 1073 · ~~`ValidationSummary/index.jsx` 1033~~ ✅ decomposed (Phase 9c-1) into rows-helpers / batch-action hook / ExportReport / BatchExportPreflight / DayStatusTable (index now 306) · ~~`DevicesStep.jsx` 825~~ ✅ decomposed (Phase 9c-3) into CamerasUsedSection / OverrideCleanupSection / ConfigVersionPanel / ElectrodeGroupsAccordion (step now 424) · `OptogeneticsFields.jsx` 826 · ~~`RecordingDaysTab.jsx` 822~~ ✅ decomposed (Phase 9c-2) into AnimalSetupCard / ExistingDataReview / DayList / DuplicateDayModal (tab now 496) · ~~`useWorkspace.js` 851~~ ✅ split into `workspaceActions` (createWorkspaceActions factory) / `useWorkspacePersistence` (status + autosave + saveNow) / `workspaceHydration` (resolveInitialWorkspace) + a pure `getAnimalDays` in `workspaceSelectors` (hook now 98) | High intrinsic load to change; many concerns per file; merge-conflict magnets. |
4141
| **~2,000 LOC legacy dead-weight** | `OptogeneticsFields.jsx` (826, imported only by `LegacyFormView`), `element/*`, `*Fields`, `LegacyFormView.jsx` — parallel to the workspace path | Every workspace UX/styling change risks needing a legacy twin; or the legacy code rots unmaintained. |
4242
| **CSS: no scoping, partial tokens** (= F6) | Global CSS, `.button-primary` redefined in 6 files, two different error icons, ~60% token adoption, mixed `.css`/`.scss` | Visual inconsistency *and* fragile edits — changing a shared style can leak across components. |

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Changed
1111

12+
- **Typed the day camera-usage helpers under strict TS (refactor only, no behavior change).**
13+
[state/cameraUsage.js](src/state/cameraUsage.ts) → `.ts`: `inferredCameraKeys` / `referencedCameraKeys`
14+
/ `resolveDayCameraUsage` (→ `Camera[]`) / `findCameraAffectedDays` and the `cameraKey` helper are
15+
typed (`unknown` for the raw animal/day inputs). The day collections are now read through the typed
16+
selectors (`getDayTasks` / `getDayAssociatedVideos` / `getDayFsGuiYamls`) instead of inline
17+
`Array.isArray(day?.x) ? day.x : []` — behavior-equivalent and removes the raw `day?.x` access.
18+
`task.camera_id` is read back as `unknown` to keep the stray-scalar tolerance honest. Two small,
19+
documented reads remain: the `FsGuiYaml` interface omits `camera_id` (a pre-existing type gap — that
20+
field is read tolerantly), and `refs.has(cameraKey(...) as string)` keeps the exact `has(null) →
21+
false` behavior without a redundant null branch. cameraUsage suite (20) + golden baselines pass;
22+
`npm run typecheck` + `CI=true` build clean; full suite 4793; e2e 104.
23+
1224
- **Typed the probe-config diff utility under strict TS (refactor only, no behavior change).**
1325
[state/configDiff.js](src/state/configDiff.ts) → `.ts`: `diffProbeConfigs` now returns the defined
1426
`ProbeConfigDiff` and consumes the newly-typed selectors (`getProbeElectrodeGroups` → `ElectrodeGroup[]`,
Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,22 @@
1818
* (the export bridge) and by the camera edit/correction blast-radius UI.
1919
*/
2020

21-
import { getAnimalCameras, getDayCamerasUsed } from './workspaceSelectors';
21+
import {
22+
getAnimalCameras,
23+
getDayCamerasUsed,
24+
getDayTasks,
25+
getDayAssociatedVideos,
26+
getDayFsGuiYamls,
27+
} from './workspaceSelectors';
28+
import type { Camera } from './workspaceTypes';
2229

2330
/**
2431
* Normalize a camera id to a comparison key so a numeric `1` and a string `"1"` match (corrupt
2532
* imports can carry either). Returns null for an absent id.
2633
*
27-
* @param {*} id
28-
* @returns {string|null}
34+
* @param id
2935
*/
30-
function cameraKey(id) {
36+
function cameraKey(id: unknown): string | null {
3137
return id === null || id === undefined ? null : String(id);
3238
}
3339

@@ -40,32 +46,33 @@ function cameraKey(id) {
4046
* uncheck a camera they only explicitly added, so the disabled decision must ignore `cameras_used`.
4147
* Shape-tolerant: non-array collections and null/undefined ids are skipped, never thrown on.
4248
*
43-
* @param {object} day - A recording-day record.
44-
* @returns {Set<string>} Normalized camera-id keys inferred from task/video/fs-gui references.
49+
* @param day - A recording-day record.
50+
* @returns Normalized camera-id keys inferred from task/video/fs-gui references.
4551
*/
46-
export function inferredCameraKeys(day) {
47-
const keys = new Set();
48-
const add = (id) => {
52+
export function inferredCameraKeys(day: unknown): Set<string> {
53+
const keys = new Set<string>();
54+
const add = (id: unknown): void => {
4955
const key = cameraKey(id);
5056
if (key !== null) keys.add(key);
5157
};
5258

53-
const tasks = Array.isArray(day?.tasks) ? day.tasks : [];
54-
for (const task of tasks) {
55-
// task.camera_id is normally an array, but tolerate a stray scalar.
56-
const ids = Array.isArray(task?.camera_id)
57-
? task.camera_id
58-
: task?.camera_id !== undefined && task?.camera_id !== null
59-
? [task.camera_id]
59+
for (const task of getDayTasks(day)) {
60+
// task.camera_id is normally an array, but tolerate a stray scalar (raw corruption), so read
61+
// it as `unknown` rather than the canonical `string[]`.
62+
const cameraId: unknown = task?.camera_id;
63+
const ids: unknown[] = Array.isArray(cameraId)
64+
? cameraId
65+
: cameraId !== undefined && cameraId !== null
66+
? [cameraId]
6067
: [];
6168
ids.forEach(add);
6269
}
6370

64-
const videos = Array.isArray(day?.associated_video_files) ? day.associated_video_files : [];
65-
for (const video of videos) add(video?.camera_id);
71+
for (const video of getDayAssociatedVideos(day)) add(video?.camera_id);
6672

67-
const fsGui = Array.isArray(day?.fs_gui_yamls) ? day.fs_gui_yamls : [];
68-
for (const protocol of fsGui) add(protocol?.camera_id);
73+
// FsGuiYaml's type omits `camera_id` (the interface predates the FsGUI camera field that
74+
// trodes_to_nwb + the `dangling_camera_ref` rule read), so read it tolerantly.
75+
for (const protocol of getDayFsGuiYamls(day)) add((protocol as { camera_id?: unknown })?.camera_id);
6976

7077
return keys;
7178
}
@@ -78,10 +85,10 @@ export function inferredCameraKeys(day) {
7885
* the cameras-used CHECKBOX disabled/hint decision use `inferredCameraKeys` instead, so an
7986
* explicitly-added camera stays uncheckable.
8087
*
81-
* @param {object} day - A recording-day record.
82-
* @returns {Set<string>} Normalized camera-id keys.
88+
* @param day - A recording-day record.
89+
* @returns Normalized camera-id keys.
8390
*/
84-
export function referencedCameraKeys(day) {
91+
export function referencedCameraKeys(day: unknown): Set<string> {
8592
const keys = inferredCameraKeys(day);
8693

8794
// UNION the explicit per-day "cameras used" set on top of the inferred references. For existing
@@ -108,26 +115,29 @@ export function referencedCameraKeys(day) {
108115
* matches ids by exact type, so it can only be MORE eager to block than this String-normalized
109116
* resolver), so a dangling reference never reaches a real export file.
110117
*
111-
* @param {object} animal - The owning animal (read shape-safely via `getAnimalCameras`).
112-
* @param {object} day - The recording day.
113-
* @returns {Array<object>} The day-used camera objects, in catalog order.
118+
* @param animal - The owning animal (read shape-safely via `getAnimalCameras`).
119+
* @param day - The recording day.
120+
* @returns The day-used camera objects, in catalog order.
114121
*/
115-
export function resolveDayCameraUsage(animal, day) {
122+
export function resolveDayCameraUsage(animal: unknown, day: unknown): Camera[] {
116123
const refs = referencedCameraKeys(day);
117124
if (refs.size === 0) return [];
118-
return getAnimalCameras(animal).filter((camera) => refs.has(cameraKey(camera?.id)));
125+
// `cameraKey` is `string | null`, but a null key is never present in `refs` (it is skipped when
126+
// building the set), so `has(null)` is always false at runtime — the `as string` keeps that exact
127+
// behavior without a redundant null branch.
128+
return getAnimalCameras(animal).filter((camera) => refs.has(cameraKey(camera?.id) as string));
119129
}
120130

121131
/**
122132
* Blast-radius helper: the ids of the given days that reference `cameraId`. Used to enumerate the
123133
* affected days before an "apply this camera correction to the N days using it" action (the
124134
* immutable-once-referenced rule), and never folded into the single-day export helper above.
125135
*
126-
* @param {Array<object>} days - Recording-day records to scan.
127-
* @param {*} cameraId - The camera id to look for.
128-
* @returns {Array<*>} The ids of days that reference the camera, in input order.
136+
* @param days - Recording-day records to scan.
137+
* @param cameraId - The camera id to look for.
138+
* @returns The ids of days that reference the camera, in input order.
129139
*/
130-
export function findCameraAffectedDays(days, cameraId) {
140+
export function findCameraAffectedDays(days: unknown, cameraId: unknown): unknown[] {
131141
const key = cameraKey(cameraId);
132142
if (key === null) return [];
133143
return (Array.isArray(days) ? days : [])

0 commit comments

Comments
 (0)