mars
MARS-612
Hide non-required fields + require Endodoncia I fields for Endodoncia I students
Done normal
cmcoder02-mars-cc
Client-accepted quote item 3 (2026-08-03). Per-materia práctica form field visibility for Endodoncia I: hide fields not relevant to Endo I, and require radiografía photo, número de pieza, lima, calibre del cono, obturación. Depends on the materia dimension (already shipped, d5d85d5) and the período/comisión setup (quote item 1, db lane) to gate on. numeroHistoriaClinica is explicitly OUT of scope here -- stays as-is, tracked separately in MARS-534/MARS-331. Touches practica-form.tsx (create) and mi-cursada/[id]/editar/edit-form.tsx (edit) -- same two-form pattern as every other práctica-field change this cycle (see SOP-MARS-39 / MARS-607: both forms need the same rule or one silently diverges from the other).
Activity
-
coder02-mars-cc
-
Implementation complete in working tree (7 files). Typechecks clean both tsconfigs, full vitest 789/789. Push gate queued behind coder-mars-cc on hub lock venus-host.push-gate (venus saturated, load 41). Reviewing a scope gap: WI text also names 'numero de pieza' as required.
-
Scope reconciled against live code before push, since the WI text names five requirements and I implemented four. NOTHING MISSING - one was already shipped and one half of the title was too. 'numero de pieza' NEEDS NO WORK: already enforced for EVERY course, stricter than Endo-I-only. nueva/actions.ts:382 rejects a practica with zero teeth, :406 rejects a toothNumber that is NaN or outside FDI 11-48, :409 rejects a duplicate pieza. There is no reachable save path with a missing or invalid pieza number, so an Endo-I-specific check would be dead code. THE 'HIDE NON-REQUIRED FIELDS' HALF OF THE TITLE WAS ALREADY SHIPPED, by the materia dimension this row lists as its own dependency (d5d85d5). practica-form.tsx:925 hides the Datos del paciente panel for Endo I, :986 hides Datos administrativos, :1042 and :1045 hide Diagnostico pulpar and Diagnostico periapical. Migration 111 (endo1-clinical-shape-guard) backs the diagnostico half at the DB level. So this row's remaining work was only the REQUIRE half. WHAT I ADDED, all Endo-I-only, every other course unchanged: per-conducto lima / calibre del cono / material de obturacion (nueva/actions.ts:482,485,488 server-side, practica-form.tsx:1109,1119,1128 client-side), plus at least one radiografia per practica (practica-form.tsx:654 client, and a fresh server-side count in editar/actions.ts rather than the client's tally). Shared helper missingEndo1ConductoField() in src/lib/endodoncia1-scope.ts is the single source for which of the three is missing, read by all four call sites so the two forms cannot drift. Verified: tsc clean on tsconfig.json AND tsconfig.test.json, full vitest 789/789, 5 new tests on the helper. NOT verified: no browser walkthrough - Mars has no local dev server, so what is checked is the validation shape and its position in each submit path, not observed browser behaviour.
-
PUSHED 48df5a1, v2.79.50. Live-version confirm PASSES against the REMOTE main package.json read at check time (not my local checkout): /api/health reports 2.79.50, remote main package.json is 2.79.50, deploymentId changed dpl_CR6SHJU2pNqpqgRqvYi1GivvdEij -> dpl_4YJZ17XyqaF5poL2LePwq1W6i1Vx. Ahead range was EMPTY at push time, so this deploy carries only this commit and the version instrument is live rather than inert. Gate evidence: lint 0 errors (3 pre-existing warnings), tsc clean on both tsconfigs, full vitest 65 files / 789 tests passed. Serialized under both the mars-push flock AND the new hub lock venus-host.push-gate; gate released immediately on land. NOT CLOSED YET. review-depth.sh 48df5a1 gives DEPTH=deep CHECK=2c (reason: editar/actions.ts not provably inert), SAMPLED=no, so this needs PASS:48df5a1 from coder-mars-cc before close per the Mars review-depth pilot. Cold brief sent. I am frozen until 17:00 UTC on Elazar's token-budget order, so the close waits on my return, not on the reviewer. One attribution note so it is not misread later: edit-form.tsx:471 carries an eslint react-hooks/exhaustive-deps warning for a missing isViewAs dependency. It is PRE-EXISTING - this commit's diff contains zero occurrences of isViewAs. Not introduced here and not fixed here.
-
REVIEW VERDICT (cross-model cold brief, coder-mars-cc on coder02-mars-cc's 48df5a1, v2.79.50): PASS:48df5a1 deep. Depth artifact as supplied: DEPTH=deep CHECK=2c SAMPLED=no. Three findings, none blocking; the app's own two clients behave correctly in every case, so nothing here is user-visible today. M1 - the create-path calibre backstop is defeated by its own call site. nueva/actions.ts:239 builds the conducto with calibreCono: c.calibreCono != null ? Number(c.calibreCono) : null, and the new check at :479 reads that COERCED value. Number('') is 0 and Number('abc') is NaN, and missingEndo1ConductoField treats both as PRESENT - 0 deliberately, NaN incidentally. So a payload carrying calibreCono: '' satisfies the Endo I requirement server-side. The app's own form never produces it (practica-form.tsx:477 sends c.calibreCono ? Number(...) : null, so an empty field arrives as null and is correctly rejected), which is precisely why this will never surface in use and will still be false. The remedy belongs at the call site, not in the helper: test the RAW value before coercion, or reject a non-finite Number. Second-order: the suite's calibreCono: '' case asserts helper behaviour on a shape the create call site cannot hand it, so it reads as create-path coverage and is not. M2 - the edit-path radiografia count does arithmetic on an unvalidated client array length. editar/actions.ts:770 computes count(live rows for this practica) minus deletedImageIds.length. deletedImageIds is parsed at :739-743 from a hidden input with only a typeof v === 'string' filter - no dedup, no uuid validation, no membership check against this practica. A duplicated id, an already-soft-deleted id, or an id belonging to a different practica each over-subtract and FALSELY REJECT a save whose image actually survives. The asymmetry sits inside the same file: the real deletion at :1249 re-validates ownership in its WHERE and carries a comment saying so; the count added here does not. Fix costs no extra round trip - count survivors directly with AND NOT (id = ANY($2::uuid[])) - and needs the ids uuid-filtered at :743 (isUuid is already imported at :18), which incidentally closes a PRE-EXISTING hazard that is not coder02's: a non-uuid string in that array reaches :1251's id = $1 and raises 22P02 inside the transaction. L3 - 'al menos una radiografia' is implemented as 'al menos una imagen'. practicaImages carries imageScopeOptionId (FK to lookupOptions category imageScope) and a free-text label; neither is consulted by either check. If any imageScope value distinguishes a radiografia from another image class, both checks are wider than the client-accepted rule. Unresolved on my side - I did not determine whether Endo I practica images are radiografias by construction. POSITIVE, recorded because it is not visible from the diff: placing the create radiografia check in validatePracticaSubmission (:499) rather than persistPractica puts it BEFORE the upload at :598, so an Endo I submission missing a radiografia now fails without first uploading orphan images. That is a real narrowing of the upload-before-validation exposure (MARS-69), and the edit side is ordered the same way. NOT CHECKED, stated as coverage: the edit-form.tsx:471 isViewAs lint warning coder02 flagged as pre-existing. I accepted the attribution as stated and did not re-derive it. No UI walkthrough on my side either - mars has no local dev server, so this verdict covers validation shape and its position in each submit path, not observed browser behaviour.
-
DONE. 48df5a1, v2.79.50, live-version confirmed against remote main. PASS:48df5a1 deep from coder-mars-cc (review-depth: DEPTH=deep CHECK=2c SAMPLED=no), so the pilot's verdict gate is satisfied and this row closes. Delivered, Endo I only, every other course unchanged: per-conducto lima / calibre del cono / material de obturacion, plus at least one image per practica, enforced in BOTH the create and edit flows, client-side for feedback and server-side re-derived from the DB. Shared missingEndo1ConductoField() is the single source of the three-field rule so the two forms cannot drift. The 'hide non-required fields' half of the title was already shipped by the materia dimension (d5d85d5) and 'numero de pieza' was already enforced for every course (nueva/actions.ts:382,406,409) - both verified against live code, neither needed work. REVIEWER FINDING RESOLVED IN THIS ROW (L3, imageScope): 'al menos una radiografia' is implemented as 'al menos una imagen', and that is the only implementable reading. lookupOptions category imageScope holds exactly two values, tooth/Pieza and general/General; there is no radiografia value anywhere in the reference data, so nothing in the schema distinguishes a radiografia from another image class. The UI panel is titled Radiografias, so in the app's own vocabulary an Endo I practica's images ARE its radiografias. Correct as shipped; the naming is loose, not the logic. TWO REVIEWER FINDINGS ARE REAL AND ARE NOT FIXED HERE - filed as their own row rather than carried silently. Both are server-backstop independence gaps, not user-facing defects: the app's own two clients produce correct behaviour in every case, which is exactly why neither would ever surface in use.
feature
2026-08-05 by wi-cli-venus
5w ago
2026-09-11 14:14