Skip to content

FE-674: Resolve pending review rows#117

Open
kostandinang wants to merge 4 commits into
graphite-base/117from
ka/fe-674-cascade-resolve
Open

FE-674: Resolve pending review rows#117
kostandinang wants to merge 4 commits into
graphite-base/117from
ka/fe-674-cascade-resolve

Conversation

@kostandinang
Copy link
Copy Markdown
Contributor

@kostandinang kostandinang commented May 8, 2026

What

Adds the Resolve button on each pending review row plus the server endpoint behind it. Closes V3.0 end-to-end.

Stacked on #116.

Why

#115 and #116 wrote and surfaced needs; users still had no way to clear them.

How

  • New POST /api/specifications/:id/reconciliation-needs/:needId/resolve — idempotent open → resolved; 404 on missing or cross-spec; 400 on bad ids.
  • Per-row Resolve button with mid-flight loading state. On success the row drops out of the list.

Out of scope

  • True undo for hard apply (resolve + restore content) — needs separate design.
  • V3.1 agent grouping.

Test plan

  • CI green
  • Trigger a hard-impact edit; click Resolve on a row; row disappears
  • Rapid double-click only fires one request

@kostandinang kostandinang force-pushed the ka/fe-674-cascade-pending-review branch from e11e4ed to 5704a0b Compare May 8, 2026 16:08
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-resolve branch from 54006af to aff3024 Compare May 8, 2026 16:08
@kostandinang kostandinang changed the title FE-674: Resolve endpoint + per-row Resolve button (V3.0 card 3, closes V3.0) FE-674: Resolve endpoint + per-row resolve (V3.0 card 3) May 8, 2026
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-resolve branch from aff3024 to ead1ecf Compare May 8, 2026 17:48
@kostandinang kostandinang self-assigned this May 8, 2026
@kostandinang kostandinang marked this pull request as ready for review May 8, 2026 17:53
@cursor
Copy link
Copy Markdown

cursor Bot commented May 8, 2026

PR Summary

Medium Risk
Adds a new write endpoint and client mutation flow for reconciliation_need resolution, including DB update semantics and query invalidation; issues could leave needs stuck open/hidden or resolve the wrong row if IDs are mishandled.

Overview
Completes side-chat V3.0’s reconciliation loop by adding an idempotent POST /api/specifications/:id/reconciliation-needs/:needId/resolve endpoint (with spec ownership validation) and a DB helper to fetch+resolve needs only when status='open'.

Updates the patch-list overlay’s Pending review section to include a per-row Resolve button that calls the new API, disables/shows “Resolving…” while in flight, and refreshes the open-needs query after success. Adds focused server and component tests covering resolve behavior, idempotence, error cases, and UI pending-state/section hiding, and updates planning/design docs to reflect the single-action V3.0 Resolve shape.

Reviewed by Cursor Bugbot for commit 308215b. Bugbot is set up for automated code reviews on this repo. Configure here.

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented May 8, 2026

🤖 Augment PR Summary

Summary: This PR completes side-chat V3.0 by adding an idempotent “resolve reconciliation need” endpoint and wiring a per-row Resolve action into the patch-list overlay, allowing users to clear open cascade needs end-to-end.

Changes:

  • Documented V3.0’s single per-row Resolve action in docs/design/SIDE_CHAT.md and updated memory/PLAN.md to mark V3.0 completed and advance the next frontier items.
  • Added POST /api/specifications/:id/reconciliation-needs/:needId/resolve and registered it in src/server/app.ts.
  • Implemented handleResolveReconciliationNeed with numeric ID validation, spec/need ownership checks, and idempotent open→resolved behavior.
  • Introduced db.getReconciliationNeed(needId) for ownership validation.
  • Added client API helper resolveReconciliationNeedRequest(specId, needId) in src/client/lib/edit-api.ts.
  • Updated PatchListOverlay to render a Resolve button per open need, track per-row in-flight state (resolvingNeedIds), and invalidate the open-needs query on success.
  • Added server route tests covering transition, idempotence, 404/400 cases, and cross-spec protection.
  • Added UI tests covering per-row buttons, correct mutation invocation, disabled-while-pending state, and hiding the section when the last need resolves.

Technical Notes: The UI uses TanStack Query invalidation (invalidateOpenReconciliationNeeds) to refresh the queue after a resolve rather than optimistic removal.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread src/client/components/patch-list-overlay.tsx
Comment thread src/server/reconciliation-needs-route.ts Outdated
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-resolve branch from ead1ecf to b1542b0 Compare May 11, 2026 09:19
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-pending-review branch from 0a6b7d4 to 01be53b Compare May 11, 2026 09:19
@kostandinang kostandinang changed the base branch from ka/fe-674-cascade-pending-review to graphite-base/117 May 11, 2026 09:41
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-resolve branch from b1542b0 to ec73476 Compare May 11, 2026 09:41
@kostandinang kostandinang changed the base branch from graphite-base/117 to ka/fe-674-cascade-pending-review May 11, 2026 09:42
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-pending-review branch from 9b5db93 to 6bc0376 Compare May 11, 2026 09:47
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-resolve branch from ec73476 to 0fe4440 Compare May 11, 2026 09:47
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-pending-review branch from 6bc0376 to fa2e56b Compare May 11, 2026 09:49
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-resolve branch from 0fe4440 to 4f63bdd Compare May 11, 2026 09:50
@kostandinang kostandinang requested a review from lunelson May 11, 2026 09:57
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-resolve branch from 4f63bdd to cb5c12b Compare May 11, 2026 10:04
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-pending-review branch from fa2e56b to 7bade11 Compare May 11, 2026 10:04
…s V3.0)

Closes invariant I112's fifth and final clause: resolutions transition
open→resolved idempotently. Closes the V3.0 frontier item (FE-674).

Server:
- New POST /api/specifications/:id/reconciliation-needs/:needId/resolve
  (handler `handleResolveReconciliationNeed` in src/server/reconciliation-needs-route.ts).
  Idempotent — already-resolved needs return 200 with no state change.
  Returns 404 on missing need OR cross-spec mismatch; 400 on non-numeric ids.
- New `db.getReconciliationNeed(needId)` for ownership validation.

Client:
- New `resolveReconciliationNeedRequest` in src/client/lib/edit-api.ts.
- patch-list-overlay: each open need row now has a Resolve button.
  Mid-flight state per-row tracked via `resolvingNeedIds` set; button
  disabled while pending and shows "Resolving…" copy. On success, the
  reconciliation-needs query invalidates and the row drops out of the list.

Tests:
- 6 new cases in resolve-reconciliation-need-route.test.ts cover transition,
  idempotence, 404 missing, 404 cross-spec, 400 non-numeric needId/specId.
- 4 new patch-list-overlay tests cover: Resolve button per row,
  resolveReconciliationNeedRequest invocation, button disabled while
  pending, section disappears when last need resolves.

Design clarification:
- SIDE_CHAT.md §9 V3.0 row updated to note the original three-action design
  (accept-on-target / edit-target / dismiss) collapses to a single Resolve
  action for V3.0 because the open→resolved transition is the same
  regardless of intent label. V3.1 reintroduces richer kinds via the agent.

Plan reconciliation:
- PLAN.md: V3.0 moves Active → Recently Completed. Continuous workspace
  promotes to Active. V3.1 (agent-grouped resolution) moves from Horizon
  to Next, gated on V3.0 walkthrough signal for A88 validation.
- Track B dependencies updated to reflect V3.0 completed.

Verified: npm run verify (1063 tests, 0 lint warnings).

Watch:
- A88 (Path 1 sufficiency without agent) is mechanically validated; full
  validation requires outer-loop walkthrough on dense graphs.
- D139 / A88 / I113 references in SPEC.md still carry the post-merge
  numbering drift from concurrent FE-698/FE-674 merges; ln-sync cleanup
  remains opportunistic.
The async IIFE that runs resolveReconciliationNeedRequest had no catch
block, only try/finally. Request failures would propagate out of the
finally as an unhandled promise rejection — visible only in browser
console as 'Uncaught (in promise)' noise and easy to miss.

Add an explicit catch that logs with the need id so failures are
attributable. The row stays in the list (no optimistic removal) so the
user can retry; surfacing a toast is left for when a project-wide toast
mechanism lands.
The handler did a read-then-write (`getReconciliationNeed` to check
status='open', then `resolveReconciliationNeed`), which under
concurrent requests could let two callers both see 'open' and both write
`resolved_at` — repeated resolves were silently rewriting the audit
timestamp. Move the `status='open'` predicate into the UPDATE's WHERE
clause so the database enforces the transition; SQLite makes the UPDATE
atomic, so only the first wins. Drop the now-redundant read-side guard
in the route.
@kostandinang kostandinang force-pushed the ka/fe-674-cascade-resolve branch from cb5c12b to 308215b Compare May 11, 2026 10:06
@kostandinang kostandinang changed the base branch from ka/fe-674-cascade-pending-review to graphite-base/117 May 11, 2026 13:42
@kostandinang kostandinang changed the title FE-674: Resolve endpoint + per-row resolve (V3.0 card 3) FE-674: Resolve pending review rows May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant