Add review map and cross-file cursor foundation - #145
Merged
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 457747163a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
pandemicsyn
added a commit
that referenced
this pull request
Jul 18, 2026
## Summary - add a versioned shared Neon review-finding contract with stable identity, source/surface revision binding, line-range or hunk anchors, canonical severity/confidence, server-stamped provenance, and explicit lifecycle state - keep findings in bounded process-ephemeral review-surface state with atomic apply, revision-bound dismiss/clear, automatic staling, expiry cleanup, and explicit-close cleanup - expose deterministic local APIs plus Flue lookup tools/actions, with a paged model context and targeted deltas over the existing multiplexed app event stream - add safety-policy entries, Neon runtime guidance, a patch changeset, and the backend/UI workstream split in `.plans/DEVIATIONS.md` ## Contract and limits `shared/review-finding.ts` is the canonical schema-version-1 contract and the canonical source of `NeonReviewFindingSeverity`. The `ReviewFindingSeverity` name added by #145 remains compatible through a type alias to that definition. A materialized finding contains `surfaceId`, `sourceId`, `revisionKey`, file and side-aware anchor data, title/explanation, `critical | major | minor | nit` severity, optional confidence and suggested action, trusted author/model/workflow-run provenance, server-owned creation time, and `active | stale | resolved | dismissed | promoted` lifecycle vocabulary. Application-owned limits: - 50 findings per apply batch - 200 retained findings per active surface - 160-character titles - 2,000-character explanations - 500-character suggested actions and lifecycle reasons - at most 50 finding ids in an event payload - model context pages default to 25 and cap at 50 items for each of files, review order, and findings, with totals and next offsets; patch bodies are never included ## Trust boundaries and validation - HTTP and Flue inputs are Valibot validated, and the registry repeats security-relevant batch and aggregate checks for trusted internal callers. - The whole batch is preflighted before registry state changes. Duplicate ids, conflicting stable ids, unavailable files, source mismatches, revision mismatches, and aggregate overflow reject the complete batch. - Stable-id idempotency/conflict applies within one `sourceId + revisionKey`. A non-active finding from an older source/revision may be replaced by the current-revision finding under the same raw id without consuming another retention slot; same-revision conflicting content remains an atomic error. - Apply, dismiss, and clear are bound to the active surface revision. Dismiss/clear require expected `sourceId` and `revisionKey`, so delayed commands cannot mutate findings after a mounted surface changes. - Dismissal transitions only `active` or `stale` findings. Repeated dismissal is idempotent, and `resolved`/`promoted` records are not rewritten by dismissal. - Public callers cannot assert stored provenance. The local API stamps `local-api` with null model/run; Flue stamps the current `agentName`/`runId` with null model. The registry stamps `createdAt`. - State is scoped only by `surfaceId`, so two windows on one PR remain independent. Findings live only in process memory, and expiry, explicit close, and disposal remove associated state. - Finding operations cannot create GitHub comments, submit reviews, request prepared-diff revisions, or mutate prepared diffs. - Event-stream messages contain only the target surface, operation, count, revision, and bounded ids. They never rebroadcast finding text, patches, or review snapshots. ## API and Flue surface Local API: - `GET /api/review-surfaces/:surfaceId/findings` - `POST /api/review-surfaces/:surfaceId/findings/apply` - `POST /api/review-surfaces/:surfaceId/findings/dismiss` - `POST /api/review-surfaces/:surfaceId/findings/clear` Neon capabilities: - `neondeck_review_surfaces_lookup` - `neondeck_review_surface_context_lookup` - `neondeck_review_surface_navigate` - `neondeck_review_surface_findings_apply` - `neondeck_review_surface_findings_dismiss` - `neondeck_review_surface_findings_clear` ## Verification - focused review-surface, navigation, event-stream, and safety tests: 4 files / 35 tests passed - `npm run check`: 96 files / 600 tests passed - `npm run format:check`: passed - `git diff --check`: passed - initial implementation verification also passed `npm run test:integration` (9 files / 129 tests) and `npm run build:server` Coverage includes atomic invalid-batch rejection, same-scope conflicts and idempotency, old-scope stable-id replacement at the retention cap, delayed dismiss/clear rejection, active/stale/idempotent dismissal, server-stamped HTTP/Flue provenance, model-context paging and totals, multi-surface isolation, expiry/close cleanup, payload limits, and targeted finding changes. ## Remaining UI work This PR intentionally does not add Pierre/React inline rendering or promotion UI. The review UI workstream still needs to render finding anchors/provenance, add finding navigation, and provide explicit user-owned promotion into the existing GitHub draft and prepared-diff revision workflows. Those paths should own `resolved`/`promoted` lifecycle transitions and retain their existing authority checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Behavior and accessibility
The cursor model is web-independent and consumes normalized metadata only. It handles filtering, empty and missing targets, boundaries, duplicates, stale targets, renamed and removed files, canonical versus guided order, and deterministic nearest-target fallback. React state and patch parsing stay outside the shared module.
The focused PR file tree keeps Pierre search, expansion, virtualization, and selection. Review rows use visible text labels with a visible legend, full descriptive status text, and a polite screen-reader announcement when the selected file changes. Status is never communicated by color alone.
Performance impact
The initial review map is derived from the metadata-only file list plus already-loaded review threads, local draft metadata, and durable Neon report findings. It does not fetch or parse additional patch bodies. Review-map updates refresh Pierre through its public composition surface without replacing the tree model, so search, expansion, and selection state remain intact.
The deterministic fixture benchmark stayed within all retained warm targets. The 305-file profile measured an 80.9 ms tree median and 299.5 ms first-patch median. The production build completed successfully; the generated MultiFileView chunk measured 223.07 kB gzip in this build.
Validation
npm run check— 96 test files and 584 tests passednpm run format:checknpm run build:dashboardnpm run bench:review-fixtures -- --out /tmp/neondeck-review-map-fixture-baseline.jsongit diff --checkLimitations and follow-up