Skip to content

fix(review): keep overlapping revision metadata truthful - #2516

Merged
Chris0Jeky merged 7 commits into
mainfrom
issue-2458/rev-overlap
Sep 4, 2026
Merged

fix(review): keep overlapping revision metadata truthful#2516
Chris0Jeky merged 7 commits into
mainfrom
issue-2458/rev-overlap

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

Summary

  • Preserve confirmed revision responses across A -> B -> A navigation and reconcile them by revision number.
  • Publish revision count/latest only when the complete chain is proven; otherwise keep metadata unknown and issue an authoritative reload.
  • Keep the [Frontend][Review] Preserve known revision state after definite save rejection #2455 definite-rejection behavior intact and add both A1-before-A2 and A2-before-A1 regressions.

Proof

  • npm exec -- vitest --run --maxWorkers=2 src/tests/composables/useProposalRevisions.spec.ts src/tests/views/paper/review/PaperReviewView.spec.ts (179 passed)
  • npm run typecheck (passed)
  • npm exec -- eslint src/composables/useProposalRevisions.ts src/tests/composables/useProposalRevisions.spec.ts (passed)
  • git diff --check (passed)

Stacked on PR #2510 so the #2455 base lands first.

Closes #2458

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

The connector usage-limit notice is informational, not a review finding. This PR is being checked through the independent coordinator review path; no code action is requested from this notice.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Review gate (Codex credits exhausted, SC-9): one fresh-context reviewer of the delta over #2510 (7db480b..3873bb4). Verdict on the delta: SHIP — but this PR is stacked on #2510, which returned FIX-FIRST (409/404 suppress the #2448 refresh barrier), so it merges only after the base is fixed and lands; base-first, then retarget/re-prove.

Clean: cross-proposal reconciliation is correctly guarded (revision.proposalId !== proposalId, and ProposalRevisionDto carries ProposalId); gap detection requires every number 1..max; each save response issues at most one reload and the reload never touches editing or the draft; #2510's catch block is untouched.

Tracked, non-blocking (follow-up issue filed):

  • MEDIUM useProposalRevisions.ts:154-169publishPersistedRevisionMetadata bumps loadGeneration before the completeness check, so a strictly newer in-flight GET (e.g. [rev1, rev2, rev3] after another session saved rev3) is discarded and a complete-looking {1,2} chain is published as authoritative with no reload; the next edit bases on a superseded revision until the ~15 s poll moves latestRevisionId. Let the in-flight GET land and merge, or re-publish after mergeLoadedRevisions.
  • MEDIUM useProposalRevisions.ts:100-115history.invalid is a permanent latch: after one inconsistent response the proposal's metadata stays unknown for the composable's lifetime (degraded, fail-safe); reset on the next authoritative GET.
  • LOW: no spec exercises the cross-proposal guard; revisionHistoryByProposal is unbounded; the A1-before-A2 spec asserts only the final count.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Heads-up for the branch owner (coordinator): this PR's base is still issue-2455/rev-reject. Tonight #2496 was merged while its base branch (already landed as #2495) still existed, so the merge went into the dead base branch and never reached main — re-targeted as #2530. Before #2516 merges, once #2510 lands, run gh pr edit 2516 --base main and re-prove CI against main; otherwise the same trap fires.

…x/issue-2458/rev-overlap-reconcile

# Conflicts:
#	frontend/taskdeck-web/src/tests/composables/useProposalRevisions.spec.ts

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-base-update adversarial pass at head f7be6a2: SHIP on the delta over current #2510. The branch now includes the fixed 404/409 uncertainty classification from #2510 and preserves the overlap reconciliation behavior; the one expected test conflict was resolved by retaining both overlap-order cases and the new 404/409/500 boundary cases. Local proof: composable + PaperReview 183/183, npm run typecheck, npx eslint on touched frontend files, git diff --check. The previous Required Gate failure was from the stale pre-fix base; this head was pushed after reconciling onto 9123a4c. No new CRITICAL/HIGH findings.

@Chris0Jeky
Chris0Jeky changed the base branch from issue-2455/rev-reject to main September 4, 2026 10:39

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coordinator re-review after base retarget/update (head d7d006a, base main f094d09): SHIP on the #2458 delta. The live diff is limited to useProposalRevisions.ts and its spec; the #2510 base fix is now merged into main, and Smart CI Plan/Required Gate pass on this refreshed head. The earlier two medium follow-ups remain non-blocking and are not merge blockers: republish after a newer GET instead of discarding it, and reset the invalid-history latch on a later authoritative load. Local focused proof was 183/183 before the base refresh. No new correctness, security, or data-loss blocker found.

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coordinator final re-review after the current-main refresh (head b7a66ab, base f965098): SHIP. The live product diff is still only the revision-history composable and its tests; the base-only merge adds no changed seam. Smart CI Plan and Required Gate pass on this head; the remaining hosted checks are pending. The previously recorded medium follow-ups remain tracked and non-blocking. No new correctness, security, or data-loss blocker found.

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coordinator final re-review after #2556 landed (head eb864df, base b8878c9): SHIP on the #2458 delta. The live diff is limited to useProposalRevisions.ts and its spec; the launcher repair is in the base only. The refreshed hosted checks are pending. Earlier medium follow-ups remain non-blocking; no new correctness, security, or data-loss blocker found.

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coordinator second adversarial pass after base refresh (head 9f18f38f8659a9d54a6f613d280e23d8d42e2d0b, base 8ee85507b2bc0958b24de3175dee17ba7d73304e): SHIP on the #2458 delta. The merge-based refresh adds only the already-reviewed #2464 capture-generation commit to the base; this PR's live diff remains limited to useProposalRevisions and its focused spec. The persisted revision map remains gap-aware, stale GETs are invalidated, and the A1/A2 navigation/order regressions are covered. git diff --check is clean. No new CRITICAL/HIGH correctness, security, or data-loss finding.

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coordinator fresh adversarial pass after the #2521 base advance (head 28d6795fb52dc10b3597204381d85ed8a19f7858, base 245886709313f0e0984b5dc1009fcfdf91515864): SHIP on the #2458 delta. The live diff is still limited to useProposalRevisions and its focused spec; the prior cancelled Windows launcher job was unrelated to this seam and no source changes were introduced by the refresh. Gap-aware revision metadata, stale-load suppression, and overlapping A1/A2 navigation/order cases remain covered. git diff --check is clean. No new CRITICAL/HIGH correctness, security, or data-loss finding.

@Chris0Jeky
Chris0Jeky deleted the issue-2458/rev-overlap branch September 6, 2026 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Frontend][Review] Keep revision metadata truthful across overlapping A1 and A2 saves

1 participant