Conflict state shows -1, sits in the wrong place, and does not block #98
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
The problems
The 409 conflict state is wrong in three ways
(
frontend/src/pages/ReviewWorkspacePage.tsx:478-492,:150-154):because the error envelope never survives FastAPI's wrapping,
err.detailsis undefined and itdisplays "Current version is -1" every time. Fixed by the error-shape issue in v0.2.0.
image canvas, while the reviewer's eyes are on the decision button they just clicked in the
right panel.
docs/circa-ui-spec.md§7.3 and §11.1 require conflict to be a blockingstate that shows what changed. The decision buttons stay enabled, nothing shows what the other
reviewer did, and the user can immediately re-submit into a second 409.
One thing that does work, by accident: the draft inputs survive reload because
useStatedoes notre-initialize — which happens to satisfy "preserve the draft."
Why it matters
This is the one moment where two family members' work collides. A banner reporting a nonsense
version, in the wrong place, that does not stop a re-submit, reads as a bug rather than as a
safety feature — and undermines trust in the mechanism that protects their work.
Scope
GET /photos/{id}plus the latest decision after reload.Done when
References
frontend/src/pages/ReviewWorkspacePage.tsx:478-492,150-154docs/circa-ui-spec.md§7.3, §11.1Depends on: the native error shape issue in v0.2.0.
Picking this up. Probed first, as the issue's own dependency note invites — two of the five done-when items are already satisfied, and by the work this issue said it was waiting on.
Already fixed, and asserted:
conflictVersionnow readserr.details.current_versionwith an explicit three-valued result — a number,nullfor a conflict the server could not name, orundefinedfor "not a conflict". The "Current version is -1" this issue was filed about is gone, and the conflict journey assertsCurrent version is \d+on a real browser against a real backend.blocked: conflict !== nullacross every versioned write, and the same journey asserts that Approve, Dispute, Needs Review, Skip and Flag Rescan are all disabled after a refusal — while deliberately leaving the AI and OCR reruns enabled, since they carry noreview_versionand cannot conflict.Still wrong, and one of them is now worse than when this was filed.
"It appears in the wrong place." The banner is still absolutely positioned over the image canvas. #97 has since docked the decision controls to the bottom of the right panel, so the banner is now about as far from the reviewer's eye as the layout allows: they click Approve at the bottom right and the explanation appears top left, over the photograph. Moving into the dock, directly above the decision form.
"The reviewer can see what the other person changed." Not built. Everything needed is already on the wire —
DecisionOutcarriescreated_by_display_name,decision_type, the date fields andcreated_at— so this is a fetch and a sentence, rendering something likeAnna approved 1983 (year), 2 minutes ago. If that fetch fails it will say nothing rather than render half a sentence, since a conflict banner is the worst possible place to guess."The draft survives, by design." The issue is right that this works by accident —
useStatesimply does not re-initialise. It will get a comment saying it is load-bearing and a test that fails if someone later "tidies" it into auseEffectthat resets on prop change.Being built alongside #99 and #100, which touch the same region of the same file.
Done in
97d5cdc, with #99 and #100. CI run #60 green on all three jobs.Done when:
On the sentence naming the other reviewer, two decisions worth recording.
It reads
Anna Whitcombe chose Approve — 1983 (year), 2 minutes ago. "chose", not "approved":labels.tskeeps decision names imperative on purpose — the history is a list of what reviewers pressed — so a past-tense verb here would mint the second vocabulary that file exists to prevent. Its own docstring supplied the wording: the label is "the same word they chose".More importantly, the line is suppressed when the newest decision predates the conflict. The write that beat the reviewer may have been a note edit or a rescan flag, not a decision at all — in which case the latest decision could be months old, and naming it would tell the reviewer that Anna's approval from March is why their save just failed. That is a confident false statement about the archive's reasoning, which is #78's failure in a place a reviewer is already unsettled. The guard compares
review_version_at_submissionagainst the photo's current version. A skip is handled too: it carries no date, and rendering "Undated" for it would put a judgement in the sentence its author did not make.Draft preservation is now deliberate. It worked because
useStatedoes not re-initialise — true, load-bearing, and entirely undocumented. There is now a comment saying so and a test asserting the typed dates survive a 409, so a later tidy-up into a prop-syncing effect fails rather than silently losing a reviewer's work at the one moment they are most likely to have typed something they care about.One cost of the move, since #97 docked the decision form.
.right-panel-dockis capped at 60% height and now shares that space with the banner, so on a short viewport a reviewer may scroll within the dock to reach Approve after a conflict. Every versioned button is disabled at that moment anyway, so it is acceptable — but it is a consequence of putting the banner where this issue asked for it, and worth knowing rather than discovering.