Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/steady-prepared-refresh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'neondeck': patch
---

Keep an applied prepared-diff review and its approval context visible when a background revision metadata refresh fails.
14 changes: 14 additions & 0 deletions .plans/DEVIATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ Use this format:
- Follow-up: What remains, who/what should handle it, or `None`.
```

## 2026-07-18 - Diff Review Phase B Completion Audit

- Roadmap item: Diff Improvements Plan / Phase B guided review completion audit
- Decision: Mark Phase B complete after independently reconciling the shared contracts and all three live review surfaces. Correct the one bounded gap found by keeping the applied prepared-diff review and its approval/recovery context mounted when a background metadata refresh fails, while showing the refresh error. Stop implementation at Phase B; do not start Phases C–E.
- Reason: The merged source/revision, navigation, finding, promotion, and refresh work satisfies the Phase B acceptance criteria, but the prepared surface's unconditional refresh-error return violated the availability-versus-application contract even though the Kilo surface already preserved applied data.
- Follow-up: Phases C–E remain later work exactly as recorded in `.plans/DIFF_IMPROVEMENTS_PLAN.md`.

## 2026-07-18 - Worktree Fingerprint Poll Content Bound

- Roadmap item: Diff Improvements Plan / Phase B revision-aware live refresh performance
- Decision: Describe the 30-second prepared/Kilo fingerprint poll as cadence- and query-scope-bounded only. Retain the current full-content hashing in `gitWorktreeRevision`; do not add caching or a hard byte/time bound during this audit.
- Reason: The poll does not load patch bodies, but it hashes every changed regular file in full and is therefore byte- and time-unbounded for pathological huge changed files. Changing that behavior affects revision identity and freshness guarantees and requires lead/user discussion rather than a risky audit-time optimization.
- Follow-up: Discuss fingerprint caching or explicit byte/time policy before implementation; until then, retain this as a known deferred performance limitation and do not claim byte-bounded content work.

## 2026-07-18 - Diff Review Performance Reconciliation

- Roadmap item: Diff Improvements Plan / transition from Phase A to Phase B
Expand Down
12 changes: 9 additions & 3 deletions .plans/DIFF_IMPROVEMENTS_PLAN.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# Diff Improvements Plan

Status: active; Phase A implementation is complete in PR #143, Phase B implementation is complete through step 5 pending the final Phase B audit, and the specialized PR review performance workstream is complete for now with measured misses explicitly deferred
Status: active; Phase A is complete in PR #143, Phase B is complete after the final audit, implementation is paused at the user-selected Phase B stopping milestone, Phases C–E remain planned, and the specialized PR review performance workstream is complete for now with measured misses explicitly deferred

Final Phase B audit note (2026-07-18): the shared source, surface, navigation, finding, promotion, and refresh contracts and the focused GitHub PR, prepared-diff, and prepared-backed Kilo/worktree surfaces were independently reconciled against every Phase B acceptance criterion and the changes in PRs #145, #146, #149, #150, #152, #153, and #154. Source/revision binding, targeted invalidation, availability versus application, dirty-state guards, explicit apply, orientation preservation/degradation, stale finding/draft trust, promotion authority boundaries, accessible navigation, and the absence of Phase C behavior are covered by focused unit, API, and component tests. The audit corrected one bounded gap: a failed background prepared-diff metadata refresh now leaves the applied review and its approval/recovery context mounted while reporting the refresh error. No Phase C, D, or E work was started.

Final Phase B performance note (2026-07-18): the lead's Node 26.4.0 rerun of `npm run bench:review-fixtures` recorded large committed-PR fixture medians of 41.9 ms for the tree, 163.7 ms for the first patch, and 0 ms for the in-process thread projection, all within the 500/1,000/500 ms fixture budgets; this harness exercises `pr-local-diffs` only. A separate 305-changed-file worktree approximation measured at the pre-final PR #154 measurement commit `aa8716783874fdf9c38bfa5fdd396b00df779788` (120 modified, 30 deleted, 25 renamed, 130 added) exercised the production Phase B step 5 paths. Across five warm Node 26.4.0 arm64 samples, repo/prepared unscoped metadata medians were 140.9/137.0 ms and repo/prepared scoped active-patch medians were 179.1/177.2 ms, all within the 500 ms tree and 1,000 ms first-patch budgets. It exercised `readRepoDiff`, `readPreparedDiffChangedFiles`, `readStableDiffMetadata`, `gitWorktreeRevision`, and expected-revision checks before and after scoped patch reads. The final PR #154 change after that measurement affected prepared-summary stable-read coverage, not those measured paths.

Progress note (2026-07-18): the specialized large-PR work now has real registered-PR measurements, stable review-thread identity, bounded local metadata reuse, active-patch priority, and passing first-patch and warm review-thread browser budgets. The workstream is complete for now. Production tree visibility, the one-time cold-object fetch, and uncached review-thread latency still miss their retained budgets and remain explicit future follow-ups in `.plans/PR_REVIEW_PERF_PLAN.md`; those misses have not been reclassified as passes.

Expand All @@ -18,9 +22,11 @@ Finding note (2026-07-18): Phase B step 3 is complete. The focused PR workbench

Promotion note (2026-07-18): Phase B step 4 is complete. The versioned source and finding contracts now declare bounded destination metadata, and one source/revision/surface/finding-bound API validates lifecycle, capability, exact line or resolved-hunk anchor, confirmation, and durable target before marking a finding promoted. GitHub findings seed the existing local review draft/comment store with preserved single- or multi-line anchors and Neon provenance; submission remains a separate existing action. Prepared and prepared-backed Kilo findings reuse the existing typed prepared-diff revision request transition, retain its authority/approval/recovery path, and cannot start a revision run. Exact retries reuse the recorded destination, target failures remain retryable, delayed completions cannot regress a newer lifecycle, and only the targeted surface receives the bounded lifecycle event. The focused PR workbench and prepared/prepared-backed Kilo viewers expose descriptive, pending-safe controls and retain promoted findings as history while active counts drop; unsupported Kilo results explain that findings remain local-only. Each promotion loads at most one requested patch. Prepared promotion also recomputes the current worktree revision from changed-file identity metadata immediately before transition, without eagerly loading the changeset's patches.

Refresh note (2026-07-18): Phase B step 5 is complete pending the lead-owned final Phase B audit. GitHub file lists and patches remain head/base-SHA bound, while prepared and Kilo/worktree metadata and per-file patch reads now carry and enforce the authoritative worktree fingerprint. Late revision responses are rejected; patch caches remain immutable under revision-keyed entries. The shared review-surface snapshot publishes bounded availability, pause reasons, application state, and preserved/degraded/failed orientation outcomes, and the multiplexed app event stream targets source revisions by source/repository/worktree/PR identity. Mounted prepared and Kilo metadata also performs a bounded 30-second fingerprint check so external worktree edits become visible without loading patch bodies. Clean surfaces may apply automatically; dirty editors, re-anchor/revision flows, mutations, stale drafts, and active selections pause automatic application, with deliberate application available only where the mounted state can be preserved. GitHub local draft-head validation remains authoritative, stale findings stay historical, exact rename metadata preserves moved files, and removed targets use a deterministic nearest review-order neighbor. Static retained sources explicitly remain static. A focused 305-file fixture loads only the active Kilo patch during refresh, and exact metadata invalidation leaves all 305 cached old-revision patch entries reusable and unrelabeled. The retained Node 26.4.0 arm64 fixture harness was rerun with large-case medians of 75.6 ms for the tree, 284.7 ms for the first patch, and 0 ms for thread projection; all targets pass. These measurements are recorded here while `benchmarks/results/review-fixture-baseline.json` remains unchanged to preserve its historical samples.
Refresh note (2026-07-18): Phase B step 5 is complete and the final Phase B audit confirmed its acceptance criteria. GitHub file lists and patches remain head/base-SHA bound, while prepared and Kilo/worktree metadata and per-file patch reads now carry and enforce the authoritative worktree fingerprint. Late revision responses are rejected; patch caches remain immutable under revision-keyed entries. The shared review-surface snapshot publishes bounded availability, pause reasons, application state, and preserved/degraded/failed orientation outcomes, and the multiplexed app event stream targets source revisions by source/repository/worktree/PR identity. Mounted prepared and Kilo metadata also performs a bounded 30-second fingerprint check so external worktree edits become visible without loading patch bodies. Clean surfaces may apply automatically; dirty editors, re-anchor/revision flows, mutations, stale drafts, and active selections pause automatic application, with deliberate application available only where the mounted state can be preserved. GitHub local draft-head validation remains authoritative, stale findings stay historical, exact rename metadata preserves moved files, and removed targets use a deterministic nearest review-order neighbor. Static retained sources explicitly remain static. A focused 305-file fixture loads only the active Kilo patch during refresh, and exact metadata invalidation leaves all 305 cached old-revision patch entries reusable and unrelabeled. The retained Node 26.4.0 arm64 fixture harness was rerun with large-case medians of 75.6 ms for the tree, 284.7 ms for the first patch, and 0 ms for thread projection; all targets pass. These measurements are recorded here while `benchmarks/results/review-fixture-baseline.json` remains unchanged to preserve its historical samples.

Refresh limitation qualification (2026-07-18): “bounded 30-second fingerprint check” above refers only to cadence, changed-path query scope, and the absence of patch-body loading. `gitWorktreeRevision` hashes the full content of every changed regular file, so the fingerprint poll remains byte- and time-unbounded for pathological huge changed files. Phase B does not claim byte-bounded fingerprint work. Adding caching or a hard byte/time limit is explicitly deferred until the lead and product owner discuss the identity/truthfulness tradeoff.

Current sequencing note (2026-07-18): Phase B is the stopping milestone. Complete step 5, then perform a Phase B completion audit and pause implementation. Preserve Phases C–E below as planned future work; do not treat the pause as completion or deletion of those items.
Current sequencing note (2026-07-18): Phase B is the stopping milestone. Complete step 5, then perform a Phase B completion audit and pause implementation. That sequence is now complete and implementation is paused. Preserve Phases C–E below as planned future work; do not treat the pause as completion or deletion of those items.

Related plans:

Expand Down
29 changes: 29 additions & 0 deletions .plans/FRONTEND_STATIC_REVIEW_20260715.html
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,22 @@ <h2 id="summary-title">Executive summary</h2>
</p>
</div>

<div class="gate completion-note">
<div class="gate-mark">PHASE B COMPLETE</div>
<p>
<strong>Guided-review outcome (2026-07-18):</strong> PRs #145, #146, #150, #152,
#153, and #154 now provide one revision-bound review model, visible and keyboard
navigation, typed inline Neon findings, explicit promotion into existing draft or
revision-request workflows, and guarded live refresh across the focused GitHub PR,
prepared-diff, and prepared-backed Kilo/worktree surfaces. The final audit corrected
one remaining availability/application gap: a failed background prepared-diff
metadata refresh now reports the error without unmounting the applied review or its
approval/recovery context. PR #149 supplied the sandbox-safe test runner used by the
landed test coverage. Phase B is the stopping milestone; Phases C–E remain later and
were not started.
</p>
</div>

<div class="stats" aria-label="Finding counts by severity">
<div class="stat"><span class="label">P0 BLOCKING</span><strong class="p0">0</strong></div>
<div class="stat"><span class="label">P1 MAJOR</span><strong class="p1">11</strong></div>
Expand Down Expand Up @@ -1029,12 +1045,24 @@ <h2 id="performance-title">Performance first: discussion report</h2>
<tr><td>Real PR · prioritized requests</td><td>1 thread request · 4 patch requests</td><td>381 kB thread · 724 kB total API</td><td>0 aborted · last API response 2,021 ms</td><td>Active read settles before background fanout</td></tr>
<tr><td>Real PR · lean threads, uncached</td><td>639 ms tree · 924 ms first patch</td><td>58 kB thread · 401 kB total API</td><td>1,511 ms threads · 16.8 ms post-response render</td><td>Payload fixed; GitHub latency remains</td></tr>
<tr><td>Real PR · lean threads, warm</td><td>642 ms tree · 934 ms first patch</td><td>530 ms traced LCP · 0.00 CLS</td><td>459 ms threads · 6.8 ms backend cache hit</td><td>Completed · warm thread target passes</td></tr>
<tr><td>Phase B fixture · large committed PR</td><td>41.9 ms tree · 163.7 ms first patch</td><td>305-file deterministic fixture</td><td>0 ms in-process thread projection</td><td>Pass · <code>pr-local-diffs</code> harness only</td></tr>
<tr><td>Phase B B5 · repo worktree</td><td>140.9 ms metadata · 179.1 ms active patch</td><td>305 changed files · five warm samples</td><td>Expected-revision pre/post checks</td><td>Pass · production B5 functions</td></tr>
<tr><td>Phase B B5 · prepared worktree</td><td>137.0 ms metadata · 177.2 ms active patch</td><td>120 modified · 30 deleted · 25 renamed · 130 added</td><td>Stable metadata + full-content fingerprint</td><td>Pass · production B5 functions</td></tr>
<tr><td>Real PR · cold local / GitHub</td><td>4,978 / 3,894 ms</td><td>295 / 765 kB metadata</td><td>0 / 1 truncated files</td><td>Both miss cold target; local is complete</td></tr>
<tr><td>Production initial preload</td><td>194.6 kB gzip</td><td>157.7 kB entry · 21.6 kB ccount</td><td>Focused-review wrapper 0.77 kB gzip</td><td>Track; no budget miss declared</td></tr>
</tbody>
</table>
</div>

<div class="callout" style="margin-bottom: 18px">
<strong>Phase B fingerprint limitation.</strong> The prepared/Kilo poll is bounded to a
30-second cadence and changed-path metadata query, and it does not load patch bodies.
Its content work is not byte- or time-bounded: <code>gitWorktreeRevision</code> hashes
every changed regular file in full. Pathological huge changed files remain an explicit
deferred performance limitation; caching or a hard bound requires lead/product-owner
discussion because it changes revision identity and freshness semantics.
</div>

<div class="callout" style="margin-bottom: 18px">
<strong>Direct Chrome evidence.</strong> The 10,000-line baseline reached 40,089
browser-reported elements, one parent with 10,000 children, a 119 ms layout, and 143
Expand Down Expand Up @@ -1367,6 +1395,7 @@ <h2 id="actions-title">Recommended sequence</h2>
<li class="completed-action"><span class="completion">COMPLETED · PR #140</span> <strong>P2 · Editor state ownership:</strong> keep new-comment, draft-edit, and thread-reply bodies scoped to their editor generation so older async completions cannot clear newer work.</li>
<li class="completed-action"><span class="completion">COMPLETED · FINAL PASS</span> <strong>P3 · Final pass:</strong> tabular metrics, timer cleanup, operation-scoped status, stable finding IDs, and config tightening. Suggested command: <code>$impeccable polish</code>.</li>
<li class="completed-action"><span class="completion">COMPLETED · CLOSING GATE</span> <strong>Re-audit:</strong> repeat the static audit, then run focused runtime and accessibility checks against the agreed fixtures.</li>
<li class="completed-action"><span class="completion">COMPLETED · DIFF PHASE B AUDIT</span> <strong>Guided review stopping handoff:</strong> reconcile source/revision consistency, navigation and accessibility, typed finding trust, explicit promotion, and guarded refresh across GitHub, prepared, and prepared-backed Kilo surfaces; retain measured misses and pause before Phases C–E.</li>
</ol>
<p class="section-note">
You can ask to run these one at a time, all at once, or in any order. Re-run
Expand Down
38 changes: 38 additions & 0 deletions .plans/PR_REVIEW_PERF_PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,44 @@ retained in `benchmarks/results/review-fixture-baseline.json`. This shared
fixture evidence complements, rather than replaces, the immutable real-PR and
production-browser evidence below.

## 2026-07-18 final Phase B audit evidence

The lead reran `npm run bench:review-fixtures` on Node 26.4.0. The large
committed-PR fixture recorded 41.9 ms tree, 163.7 ms first-patch, and 0 ms
in-process thread-projection medians, inside the retained 500/1,000/500 ms
fixture budgets. This deterministic harness exercises `pr-local-diffs` only;
it does not replace the retained production-browser misses below.

A dedicated 305-changed-file worktree approximation measured at the pre-final
PR #154 measurement commit `aa8716783874fdf9c38bfa5fdd396b00df779788` on Node
26.4.0/arm64 exercised the production Phase B step 5 functions. The fixture
contained 120 modified, 30 deleted, 25 renamed, and 130 added files. Five warm
samples produced:

| Production B5 path | Median | Range | Retained budget / verdict |
| ---------------------------- | -------: | -------------- | ------------------------- |
| Repo unscoped metadata | 140.9 ms | 134.8–143.4 ms | tree <500 ms / pass |
| Prepared unscoped metadata | 137.0 ms | 134.4–139.0 ms | tree <500 ms / pass |
| Repo scoped active patch | 179.1 ms | 177.7–188.1 ms | patch <1,000 ms / pass |
| Prepared scoped active patch | 177.2 ms | 173.9–181.1 ms | patch <1,000 ms / pass |

The approximation exercised `readRepoDiff`,
`readPreparedDiffChangedFiles`, `readStableDiffMetadata`,
`gitWorktreeRevision`, and expected-revision checks before and after scoped
patch reads in `readRepoDiff` and `readPreparedDiffFileDiff`. The final PR #154
commit after the measurement changed only prepared-summary stable-read code
and coverage, not those measured paths. Machine-local raw evidence was retained
at `/private/tmp/neondeck-pr154-b5-results-exact-aa87167.json`.

The mounted prepared/Kilo 30-second fingerprint polling cadence and changed-path
metadata query are bounded, and patch bodies are not loaded by the poll.
Content work is not byte- or time-bounded: `gitWorktreeRevision` hashes the full
content of every changed regular file. Pathological huge changed files can
therefore make a poll expensive. No caching or hard byte/time bound was added
during the Phase B audit because that changes revision identity and freshness
semantics; it remains an explicit deferred performance limitation for lead/user
discussion.

## 2026-07-17 reconciliation

PR #84 implemented phases 1–5 plus the original synthetic 305-file harness.
Expand Down
Loading