Let a reviewer adopt an evidence date into the decision form in one click #96
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Severity: HIGH - highest leverage single change in the audit
The problem
The decision form initializes from
current_effective_date_*, which is populated only by priordecisions — never by evidence. So for every fresh photo the form is empty, and the reviewer
manually re-types a date that is displayed right there in the evidence panel, two rows above.
That is two full
type="date"entries (month, day, year, twice) plus a precision select, perphoto, thousands of times. The product's stated first principle is "evidence-first," and the UI
makes the reviewer transcribe the evidence by hand.
The fix
Make an evidence row clickable to adopt its
date_low,date_high, andprecisioninto thedecision form. One click replaces two date entries and a select.
Scope
the decision was based on — valuable later for the constraint explanation UI (#20).
day.
Why this matters most
Of everything in the audit, this is the highest ratio of reviewer time saved to implementation
effort. It is deliverable independently of the navigation work, though the two together transform
the loop.
Done when
References
frontend/src/pages/ReviewWorkspacePage.tsx:120-239(DecisionForm), evidence list belowdocs/circa-ui-spec.md(evidence-first principle)Picking this up. The premise checks out exactly as written, and there is one scope decision in it that the issue leaves open.
Confirmed.
DecisionFormseeds its three date fields fromphoto.current_effective_date_*, andservices/projections.foldsets those only from a prior decision — evidence never reaches them. So for a photograph nobody has decided yet the form is genuinely empty while the evidence panel is displaying a date two rows above it, and the reviewer retypes it by hand: twotype="date"entries and a precision select, per photograph, across the collection. For an application whose first principle is evidence-first, the UI currently makes a person transcribe the evidence.The open question was where to record what a decision was adopted from, and it is a schema question rather than a UI one. The issue says "the decision's rationale or provenance".
DateDecisionhasrationale, free text, and no provenance field at all — so those two options are not equivalent, and the choice decides whether #20's constraint-explanation UI can query the link or has to parse prose.Settled with @rbrooks: a structured
adopted_evidence_id, a nullable FK fromdate_decisiontodate_evidence, in migration 013 and surfaced onDecisionOut. Two reasons it wins over writing text intorationale:rationaleis what a reviewer typed. This codebase is careful about that distinction everywhere else — notes and comments are what a relative said about a photograph, and machine-generated text does not go in them. Prepending "Adopted from Back OCR" would either overwrite the reviewer's words or be silently mixed with them.It is a new column on a write route, so it will trip
test_api_contract.pyand thereview_versionsweep intest_concurrency_protocol.pyuntil it is declared. That is the intended behaviour of those tests, not an obstacle.On "adopts without inventing precision", which is the subtlest of the four done-when boxes.
DateEvidencecarriesdate_low,date_highandprecisionalready, so the honest adopt is a straight copy of all three rather than a computation. The case to get right is evidence with araw_valuebut no parsed dates — an OCR row readingJune 83?that nothing has normalised. Such a row has nothing to adopt, and it must not be clickable at all: filling the form with a fabricated range from an unparsed string would put a date into the archive that no evidence established, which is exactly #78's failure in the place it would do the most damage. Those rows will be inert and visibly so.Scope: click and keyboard-activate an evidence row to adopt its dates and precision into the form; mark which row the current draft came from; clear that mark when the reviewer edits the values, since it is then no longer that evidence's date; send
adopted_evidence_idwith the decision; show the adopted source in the decision history. Rows with nothing to adopt are not activatable.Done in
73bea9b(auto-closed by the commit trailer). CI run #58 green on all three jobs.Done when:
aria-pressedin the panel, recorded asadopted_evidence_idondate_decisionprecisionis copied rather than derived from the range1206 backend tests (was 1186), 132 frontend (was 121), 7 e2e.
Three things found in the building that are not in the issue.
The migration's obvious form fails halfway.
op.add_columncarrying aForeignKeyraisesNotImplementedError: No support for ALTER of constraints in SQLite dialect— but only after the column has been added, so the schema ends up with the column, without the constraint, and the migration marked failed. It needsbatch_alter_table, which this project otherwise avoids because on SQLite it is create-copy-drop-rename and is how a table's other objects quietly disappear.date_decisionwas checked for triggers before reaching for it — it has none, #67's are onaudit_event— and the four foreign keys, both indexes and those triggers are now asserted to survive rather than assumed to.A row with nothing to adopt had to be made genuinely inert, not merely unhelpful. Evidence carrying a
raw_valueand no parsed dates — an OCR read ofJune 83?— has nobuttonrole and no tabindex at all. Adopting from it would have filled the form from an unparsed string, putting a date in the archive that no evidence established.One of the new tests passed for the wrong reason. The viewer-cannot-decide case expected 403 and got one — from the CSRF origin guard, before the route was reached — so it would have passed with the role floor deleted. Every POST in that file now carries an
Originheader. Worth knowing for anyone writing a test that asserts a refusal here: a 403 is not evidence that the thing you are testing refused it.Verified by mutation: dropping the ownership check fails all eight security cases, removing the per-photograph reset fails the leak test, and making every row adoptable fails the unparsed-row test.
A note for #20 and #108.
adopted_evidence_idis deliberately singular and nullable — it records that a reviewer took this date from that row, which is a narrower claim than the multi-parent provenance #108 is about. It should be a useful first input to that work rather than something #108 has to unpick, but it is not a substitute for it.And one for #97, which is next. #96 changes what that issue is about. The evidence panel is no longer a reference display a reviewer reads — it is now the primary control for dating a photograph, the thing you click to make a decision. So "evidence sits below the fold" has gone from a legibility complaint to a claim that the main interaction is off-screen.