mirror of
https://github.com/backnotprop/plannotator.git
synced 2026-09-14 14:17:26 +08:00
feat/auto-viewed
13 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michael Ramos
|
d0c32c8863 |
fix(review): preserve dragged diff ranges on compact touch before commenting (#1333)
* feat: add mobile touch range selection prototypes * docs: redirect mobile range selection spike * revert: remove command-mediated touch range prototype * docs: align mobile line selection with DiffsHub * feat: preserve mobile diff ranges before commenting * fix(review): repaint a preserved mobile range on a second drag A preserved range leaves pendingSelection non-null, so DiffViewer hands Pierre a defined selectedLines and Pierre switches to controlled selection: updateSelection then only records a proposed range and leaves painting to the host. With no change handler wired, a second drag never repainted, so the old highlight stayed put and the finger was untracked until release. Wire onLineSelectionChange back into app state, only on compact touch, so desktop keeps an options object with no such key. Also route a null range to the toolbar host instead of swallowing it in the preserve branch, mirroring AllFilesCodeView's early return so an open composer (Ask AI included) closes with the selection it was written for. * fix(review): 44px hit area for Pierre's gutter comment button on touch With a dragged range now preserved instead of opening the composer, that button is the only way to start writing about it, and it is roughly 20px square: below the data-pn-touch-target standard the rest of the compact shell holds. Grow its invisible ::before hit area to the 44px token, leaving the glyph alone. The rule ships through the same unsafeCSS both diff surfaces already inject, which lands in Pierre's shadow root inside @layer unsafe (last in the library's layer order, so no !important). It is injected only when the shell is compact rather than gated in CSS: html:has() matches nothing from inside a shadow root, and @media (pointer: coarse) would wrongly claim a desktop with a touchscreen. * test(review): cover the compact-touch preserved range on both diff surfaces DiffViewer.compactTouchSelection.test.tsx drives the FileDiff options the component hands Pierre: a drag preserves the range instead of opening the composer and paints it through selectedLines, a second drag repaints through onLineSelectionChange, the gutter action opens the composer, a cleared range still reaches the toolbar host, and desktop keeps routing completed drags straight to the composer with no change handler at all. The last three assertions fail against the pre-fix component. The AllFilesCodeView compact test only checked what was published upward, which a range nothing paints would also satisfy; it now asserts the range reaches the CodeView props. That needs the App loop, so the mount feeds published selections back down as pendingSelection: without it the reconcile effect clears the highlight the preserve branch just painted. * chore(guides-show): regenerate viewer manifest pin for the touch selection changes |
||
|
Michael Ramos
|
64062af9a1 |
feat: Portable Guided Reviews — export, share links, agent-authored guides, guides.show (#1324)
A Guided Review can now leave Plannotator: as a single self-contained HTML file that renders exactly like the in-app guide, as an encrypted-by-default share link on guides.show, or authored by any agent through the new guide CLI. Highlights: packages/guide-viewer extracted from review-editor at the injection seam (read-only host, no third renderer); guides.show Worker with R2-backed share storage, per-IP rate limiting on creation, delete tokens hashed at rest, and 128-bit ids; portable exports pin the viewer by SRI hash with budget and manifest gates in PR CI and at deploy; two-runtime parity across Bun and Pi verified; v0.27.x saved guides load unchanged. Retention is indefinite by explicit decision, to revisit with the lean sharing refactor. Decision record: adr/decisions/007-portable-guided-reviews-20260815.md |
||
|
Michael Ramos
|
27791a6fba |
Mobile Phase 2B: Plan shell and navigation (#1303)
* feat(editor): add compact plan navigator * feat(editor): simplify compact plan chrome * fix(editor): close compact navigator after file selection * feat(editor): add compact plan review surfaces * fix(editor): hold navigator through cold file loads * fix(editor): preserve desktop diff activation |
||
|
Michael Ramos
|
64e3fa7762 |
feat(review): add compact touch review shell (#1301)
* feat(review): add compact touch review shell * fix(review): let submission dialog own initial focus * fix(review): refine compact mobile review chrome * fix(review): restore reliable mobile diff scrolling * fix(review): preserve mobile file identity * docs(mobile): specify phase 2b plan shell * fix(review): close mobile shell regressions * fix(review): restore narrow overview stacking * test(review): preserve real syntax theme resolver |
||
|
Michael Ramos
|
f387cdabde |
feat(ui): add mobile-safe touch and dialog primitives (#1300)
* feat(ui): add mobile-safe touch and dialog primitives * fix(ui): scope touch targets to compact shell |
||
|
Michael Ramos
|
e181b824cc |
Mobile-safe plan and code comment composition (#1297)
* feat: harden mobile comment composition * fix(ui): keep mobile app inside Safari viewport * docs: record physical mobile triage * fix(ui): extend plan canvas behind Safari controls * fix(ui): let mobile plans drive Safari chrome * fix(ui): release Safari top edge on mobile plans * docs: triage mobile feedback and close phase 1b * fix(ui): harden compact touch behavior |
||
|
Michael Ramos
|
d3633c9c52 |
Mobile foundation and quieter first run (#1295)
* feat: establish mobile foundation and simplify onboarding * fix(ui): finish mobile foundation cleanup |
||
|
Michael Ramos
|
d6b98f0c0d |
feat(review): Guided Review + Pi agent-job provider (#993)
* docs(adr): Guided Review ADR/spec/research + agent-provider fit studies
ADR 006 (Guided Review as a first-class feature), four implementation
spikes, synthesis, spec (iterated through preflight and review fixes),
recap, and the Flue/Pi provider-fit research syntheses.
* feat(review): Guided Review takeover + Pi agent-job provider
Guided Review (ADR 006) — a Linear-Guides-style chaptered review:
- guide agent-job provider (packages/server/guide/guide-review.ts):
schema-constrained sections (title/overview/file refs) over the live
patch, coverage-validated server-side (every changed file exactly once,
fabricated paths dropped, fail-closed on empty output), importance-first
ordering prompt with speed discipline (diff-first, no repo exploration)
- runs on claude/codex natively and cursor/opencode/pi via the marker
contract; guide-scoped low effort defaults for quicker generation
- takeover UI (packages/review-editor/components/guide/): Guide header
badge + Mod+Shift+G, full-width screen that CSS-hides (never unmounts)
the file tree/dock, empty state with inline model pickers, skeleton
loading, generating state with collapsed activity log, two-column
section cards (sticky prose column with internally-scrolling file list,
content-height diffs, review-independent collapse, peek semantics)
- annotation parity: guide diffs mount the real DiffViewer with
file-scoped handlers into the same CodeAnnotation state and feedback
export; per-section reviewed state persisted via /api/guide/:jobId
- reviewed/tour prompt speed sections; renderMarkdownProse shared between
tour and guide (fenced-block support, muted tone variant)
Pi agent-job provider:
- third MarkerEngine (pi --mode json --no-session --no-approve) with
live model-catalog discovery, thinking-level control (--thinking),
fail-closed marker parsing (Pi exits 0 on in-run errors by design)
- available for review + guide jobs from both launch surfaces
Shared UX:
- searchable, provider-grouped model pickers (auto over 12 options)
- full Claude version catalog with latest-resolving aliases labeled
- AgentControls primitives extracted from AgentsTab; cross-instance
settings sync in useAgentSettings
- Pi extension server hand-mirrors + vendor.sh entries throughout
* fix(guide): PR-993 review fixes + failure recovery ladder
Review fixes:
- coerce marker-engine title/intent at ingest (prompt-enforced output
could store non-strings and crash the takeover)
- keep placed files out of unplacedFiles (exactly-once guarantee)
- shared REVIEW_ENGINE_LABEL (fixes 'generated by pi' header; typed
exhaustiveness prevents recurrence)
- key ActiveGuide by jobId so per-guide focus state resets
- formatModel handles marker-engine tour/guide jobs (empty model chip)
- drop dead guideCodexFast; memoize estimateDiffHeight
- store + display Pi thinking level on job cards
Failure recovery ladder (auto -> one click -> manual -> regenerate):
- mechanical JSON repair pass (fences/slice/trailing-commas/bracket
balance) before any guide parse fail-closes
- failed payloads captured per job (200KB cap, per-engine extraction)
- 'Fix output' repair job: pure text transform on a schema-capable
engine (claude/codex preferred), forced low effort, lands as a
normal guide job; repairOf threaded through launch validation
- editable output panel: GET /api/guide/:jobId/output prefills a
textarea, POST /api/guide/:jobId/submit runs the same server-side
validation and opens the fixed guide; inline errors for iteration
- Pi extension mirrors + AGENTS.md endpoint/body docs
* fix(guide): string-aware mechanical JSON repair (self-review)
- terminate a dangling string literal before appending bracket closers
(truncation mid-string is the most common shape; closers appended
inside the string never parsed)
- strip trailing commas outside string literals only (the regex could
rewrite overview content, violating the repair contract)
* fix(guide): launch-snapshot validation, codex strict schema, repair persistence
PR-993 round-2 review fixes + live-test findings:
- validate guide completion (and manual repair) against the LAUNCH-time
changed-file set, snapshotted per job — switching diff/base/PR while a
guide generates no longer destroys valid output (client already
degrades stale refs per-file)
- codex strict structured output: unplacedFiles now required in the
guide schema (OpenAI 400s schemas with optional properties under
additionalProperties:false — live-confirmed, then live-verified fixed:
5 sections / 83 files placed on a ChatGPT-account codex)
- manual repair survives reload: successful /submit flips the job to
done via completeJobExternally + status guard on the endpoint
- codex model defaults -> gpt-5.5 (5.3-codex is deprecated; ChatGPT
accounts reject it) with one-shot migration of saved picks; guide
codex default reasoning stays low
- pi extension guide branch snapshots launch state (TOCTOU hygiene)
- paragraph collector no longer swallows an unspaced code fence
- tour hook backports: out-of-order fetch guard; save outside updater
- guide empty-state copy rewritten value-first
* fix(guide): PR-993 round-3 — repair-diff fidelity, failure surfacing, pi tool hardening
- repair jobs validate against the FAILED job's recorded file set (the
payload under repair references that changeset; validating against the
diff on screen at repair time re-introduced destroy-on-switch)
- takeover follows the NEWEST running guide (progress + Cancel), and a
failure newer than the shown guide surfaces as a dismissible strip
(error + Fix output + Details) instead of being silently masked
- pi jobs run with --exclude-tools edit,write (bash stays: the agent
needs git inspection; full read-only would break generation)
- tour/guide result ingestion fails closed on unexpected throw (done-
looking jobs no longer 404 their result)
- sidebar guide mode gated on file availability like the header badge
- drop orphaned DEFAULT_GUIDE_CODEX_FAST (fast mode intentionally not
offered for guide); retry owns its fetch cancellation; final
trailing-comma pass after bracket-closing in the repair ladder;
collapsed-row checkbox/expand are sibling buttons (a11y)
* fix(guide): PR-993 round-4 — callout text, guide job detail, toggle races
- single-line callouts (> [!IMPORTANT] message) keep their message — the
form our own prompt solicits rendered an empty labeled box; type now
derives from the [!TAG] capture, not a whole-line scan that mistyped
'> [!NOTE] this is important'
- job detail panel gets a guide status card (Open guide via a new
ReviewStateContext.openGuide) instead of mislabeling 'Guide Generated'
as a red Incorrect review
- marker-engine guide jobs get readable live logs (engine fallback in
the formatter lookup; provider stays 'guide')
- repair prefers the failed job's own schema-capable engine (proven
working on this machine) before falling back by binary presence
- toggleReviewed/toggleChecked: pure functional updaters + persistence
effect with seed-skip — race-proof AND StrictMode-safe (resolves the
tradeoff previous rounds accepted)
- dedups: formatDuration/ElapsedTime shared from AgentsTab; whichCmd
exported in the pi extension; SEARCHABLE_THRESHOLD imported by
InlinePicker
- new renderMarkdownProse tests (single-line/multi-line callouts,
paragraph passthrough)
* fix(guide): PR-993 round-5 — keep engine-default model option, label Pi jobs
- GuideEmptyState's marker catalogs mirror AgentsTab's per-engine
semantics: opencode/pi prepend the engine-managed Default ('' value)
to the discovered list (dropping it left saved-default users a blank
pill with no way back); cursor replaces (its list includes 'auto')
- job detail provider pill labels pi review jobs 'Pi' instead of the
'Shell' fallback
* fix(guide): PR-993 round-6 — context scoping, stale models, repair engine, staging gate
- guide takeover + auto-open scoped to the current review context: a
guide launched against PR A no longer shows over PR B, and a guide
finishing for an away context defers its auto-open until the reviewer
returns to that context (unmarked in the dedupe set on purpose)
- guide launcher reconciles saved cursor/opencode/pi model ids against
the live catalog at read time (picker + launch share one effective
value) — no more posting dead ids after an account switch
- repair prefers the failed job's OWN engine whenever its binary is
present (provably runnable; marker binaries resolved via
MARKER_ENGINES — cursor's CLI is 'agent'); claude/codex are fallback
only, killing the broken-claude repair doom loop
- per-file staging gate (canStagePath) in guide diffs AND the
pre-existing same gap in ReviewDiffPanel — committed-only files in
since-base reviews no longer offer a no-op Git Add
* fix(review): scope tour auto-open to current context (self-review)
Same cross-context gap just fixed for guides: a tour finishing for PR A
popped its dialog over PR B. Both auto-open effects now share one
jobMatchesCurrentContext helper with the same deferred-open semantics.
* fix(guide): PR-993 round-7 — complete the context-scoping story
Round 6 scoped guides to their review context; this closes the two
surfaces that scoping left dangling, plus a focus gap:
- switching to a context that already HAS a completed guide now shows
that guide: the takeover falls back to the context's newest done
guide job when activeGuideJobId belongs elsewhere (previously landed
on the empty state with the guide sitting unreachable)
- 'Open guide' affordances are context-gated everywhere they exist:
job cards hide it for cross-context guides (opening can't switch
PRs), and the job detail panel explains where the guide belongs
instead of offering a dead button; one shared
jobMatchesReviewContext predicate now backs App, GuideScreen,
AgentsTab, and the detail panel
- file-chip navigation retargets the guide's focus arbiter (scrolling
under a stationary pointer fires no pointerenter, so the annotation
toolbar stayed bound to the previous diff)
* fix(tour): retry owns its fetch cancellation; document prUrl-only scoping
- useTourData retry converts to the nonce-driven effect re-run pattern
(the round-4 backport carried the fetch guard but not the retry fix
useGuideData got in the same batch — half a backport)
- jobMatchesReviewContext now documents WHY it matches by prUrl only
and not diffScope/diffContext: guides/tours reference files and
degrade per-file when the diff shifts; scope-strict matching would
hide useful artifacts on layer/base/mode switches. Deliberate,
do-not-tighten-without-UX-decision
* fix(guide): PR-993 round-9 — worktree-aware context, guide-scoped marker settings
- jobMatchesReviewContext now compares diffContext.worktreePath for
local jobs: prUrl + worktree define WHERE the review is (matched
strictly); mode/base/scope remain WHAT VIEW (deliberately loose) —
a guide launched against worktree A no longer opens over worktree B.
Jobs predating the snapshot coalesce to the main tree.
- Cursor/OpenCode/Pi get guide-scoped model/thinking settings
(guideCursor/guideOpencode/guidePi), mirroring the existing
guideClaude/guideCodex isolation: tuning a guide's marker model no
longer silently changes the next code review with that engine.
Defaults are each engine's natural default, not seeded from review
settings (same precedent as guideClaudeEffort='low'). AgentsTab's
reconcile effects snap both surfaces against the shared catalogs.
* fix(review): keep worktree parsing browser-safe (round-9 build fix)
parseWorktreeDiffType lives in shared/review-core, which imports
node:path at module top — pulling it into App.tsx broke the vite
browser build (typecheck and tests don't catch it; only the bundle
does). Context matching now reuses App's existing hand-parsed
activeWorktreePath memo — one parse, and it's the same one that
drives the sections/tree UI, so matching aligns with what's on
screen. Also fixes a deps-array reference to the removed memo that
bundled fine (treated as a global) but would have thrown a
ReferenceError at runtime.
* docs(review): mark worktree-subtype lockstep between server parser and App copy
* fix(review): PR-993 round-10 — guide shortcut gate, settings sync race, heading markdown
- bare a/v staging/viewed shortcuts suspend while the guide takeover is
open (the dock is only CSS-hidden, so a diff panel stayed 'active'
underneath and bare keys acted on an invisible file)
- cross-instance settings sync replaces the was-last-update-remote
boolean with value comparison (lastSyncedJsonRef): the flag conflated
'a commit happened' with 'the last change was remote', so a local
edit batched with an incoming broadcast silently skipped its own
cookie write and rebroadcast
- ### headings in guide/tour prose run through renderInlineMarkdown
like every other block (backticked symbols rendered as raw tokens);
regression test added
* fix(guide): PR-993 round-11 — reveal channel for sidebar jumps, repair exit code
- new guideRevealFile channel closes the documented round-7 gap and its
AI sibling: sidebar jumps (annotation clicks, AI line citations) made
while the guide takeover is open no longer mutate the hidden dock's
active file; instead the GuideSectionCard containing the target file
expands its collapsed (reviewed) section, focuses the diff, and
scrolls to it — so jumps into collapsed sections stop silently
no-opping. Cleared on guide close/switch so keyed remounts don't
replay the last reveal.
- completeJobExternally resets exitCode to 0 (both servers): a
successfully repaired guide kept showing the failed run's Exit 1 chip
in the job detail panel
* fix(guide): clear reveal channel in-batch at guide-switch sites (self-review)
The clear-on-change effect fires after a switched guide's keyed cards
have already mounted (child effects run before parent effects), leaving
one commit where a stale reveal from guide A could expand+scroll a
same-named file's section in guide B. All three setActiveGuideJobId
sites now clear synchronously in the same batch; the effect stays as a
backstop for close and future set sites.
* fix(guide): PR-993 round-12 — titled sections, honest large-PR validation
- sanitizeGuideSection gives every surviving section a non-empty title
('Untitled section' fallback): a diffs-only section rendered as a
blank chapter inside a 'Guide Generated' job — no parse failure, so
no recovery flow. Keeping (titled) beats dropping: placed files are
not in unplacedFiles, so dropping would orphan them silently.
- guide launches on large PRs (layerPatchIncomplete) recompute
changedFiles from the local checkout (git diff --numstat
origin/<base>...HEAD, rename/binary handling) when available — the
PR-mode prompt tells the agent to read that full local diff, but the
changed-files block and validation snapshot came from the truncated
platform patch, under-listing files to the model and then dropping
its valid refs (or failing the guide) at validation. Falls back to
the partial list on any failure. Mirrored in the Pi extension server.
Round-12's P1 (Pi marker extraction reads message_end, 'should' read
text_delta) was refuted by live probe: Pi's assistant message_end
carries the fully-assembled content blocks, which is exactly what
piExtractText reads; the delta-reading code the finding cross-
referenced is the Ask-AI STREAMING provider, a different job.
* test(guide): first direct coverage for guide-review's pure logic (self-review)
The module's repair ladder, validation, and stream parsing had zero
unit tests — exercised only end-to-end through live agent runs. Pins
the behaviors the review rounds fixed: blank-title fallback (round 12),
first-placement-wins dedup, changedFiles filtering + fail-closed empty
guides, prose-only vs lost-diffs section handling, unplacedFiles
merge/dedup/fabrication-filtering, title/intent coercion, trailing-
comma and unbalanced-bracket repair, truncated stream-line recovery.
13 tests; server suite 374 → 387.
* fix(guide): PR-993 round-13 — full-stack recompute regression, flag snapshot, unplaced reveal
- the round-12 large-PR recompute now runs ONLY in layer scope: in
full-stack scope launchPatch is already a local full recompute, and
the layer diff (origin/<base>...HEAD) was the WRONG file set — it
omits earlier stack layers' files, dropping their refs at validation
or failing the guide. Introduced by round 12; caught before any
release. Both servers.
- layerPatchIncomplete is snapshotted with the other launch locals
(launchLayerPatchIncomplete): coherence with launchPatch was
positional (same sync segment) — now structural, so a future await
inserted upstream can't silently desync them. Both servers.
- the 'Everything else' bucket handles guideRevealFile: sidebar jumps
to unplaced files now focus + scroll their diff (the section-card
effect only covers placed files) — closes the residual gap noted in
the round-11 self-review
|
||
|
Michael Ramos
|
795f381ebe |
feat(review): "All changes" git-status review view (#990)
* feat(review): "Since main" git-status review view + correctness fixes
Adds a composite `since-base` diff — merge-base(origin/main, HEAD) vs the
working tree, plus untracked — as the default code-review view, rendered as a
three-section "git status" panel (Committed / Changes / Untracked) with a
Sections|Tree toggle, a first-run setup chooser, and a "baseline behind GitHub"
fetch banner. Everything normalizes to one git patch, so the diff viewer,
annotations, and feedback path are unchanged.
New diff type wired through runGitDiff / fingerprint / file-content / context /
staging / agent-context in packages/shared + both server runtimes (Bun +
Pi mirror). Default flipped from `unstaged` to `since-base`; the old
DiffTypeSetupDialog is replaced by ReviewSetupDialog (view + default-diff
chooser, screenshots, Settings access).
Includes a reviewed batch of correctness fixes:
- P0: baseBehindRemote was permanently true (rev-parse missing --verify)
- fingerprint blind to quoted/unicode untracked paths (unquote)
- sections parser: record both sides of a rename; staged-delete wins over
untracked (rm --cached collision)
- graceful degrade when merge-base can't resolve (trunk/no-remote repos)
- /api/fetch-base re-queries remote (narrow-refspec honesty)
- /api/diff/switch concurrency guard (diffSwitchEpoch) + draft rekey
- decouple remote-staleness probe from initialBase (Pi parity)
- keyboard file nav in the sections view; banner gated to base-relative modes
See adr/decisions/005-since-base-github-view-default-20260701-223706.md
* fix(review): since-base review-round hardening (11 fixes, both runtimes)
Addresses the multi-agent + PR-990 review findings:
- Sections rename parser split ` -> ` on the raw quoted porcelain token, so a
filename containing ` -> ` tore into garbage and a dirty file could render as
"Committed". Now quote-aware (splitPorcelainRename + unit tests).
- /api/diff/switch captured the epoch AFTER await req.json(), letting a
slow-body older switch overwrite a newer confirmed one. Epoch now captured
before any await; hideWhitespace committed only on win. Both runtimes.
- Unresolvable base (trunk / no origin/HEAD) no longer auto-defaults to a
degraded since-base that hides committed work — getGitContext only offers
since-base when the base ref resolves, so the default falls through to
uncommitted. Fingerprint + file-content degrade to HEAD to match the diff.
- git rm --cached no longer yields two diff entries for one path (untracked
files already in the tracked patch are dropped) — fixes wrong-file-open and
j/k nav looping in the Sections panel; also fixes latent uncommitted/unstaged.
- Settings' "Default Diff View" list now preserves the sections<->since-base
coupling (can't leave an invalid sections+classic pair).
- "Behind GitHub" banner only treats origin/* as fetchable; a bare local base
("main") is upgraded to its tracking ref at startup so Fetch can clear it.
- hashUntracked resolves untracked paths against the repo toplevel, not cwd, so
a review launched from a subdirectory isn't blind to untracked edits.
- Agent review instruction now tells agents to enumerate/inspect untracked files.
- Terminology: one "Committed changes" label; the live "Since <base>" label is
dynamic (matches the header); "Since main" kept only as product copy.
- Docs: CLAUDE.md endpoints/fields + since-base view section; ADR recap.
Verified: typecheck (all projects), bun test 1793 pass, and empirical repro of
the rm --cached dedup, trunk->uncommitted fallback, and rename parser.
* fix(review): restore "(PR view)" on the Committed-changes label (unify toward it, not away)
* fix(review): close since-base coverage gaps from PR-990 review round 2
- First-run setup no longer forces since-base on repos where it isn't available
(base ref can't resolve). getGitContext omits since-base there; the first-run
block now checks the same availability before resetting the default or showing
the chooser, so committed work isn't silently hidden on trunk/no-origin repos.
- reviewBase now includes 'since-base', so changing the base while in the
git-status view passes the selected base to /api/file-content — expandable
context is fetched from the right merge-base (was falling back to default).
- Settings now mirror ReviewSetupDialog's coupling exactly: Sections ⇒ force
since-base; Tree ⇒ leave the diff (Tree + since-base is valid and now
saveable); classic diff ⇒ Tree; since-base diff ⇒ leave the view. Removes both
coercions that previously discarded a supported preference.
* fix(review): PR-990 review round 3 — fingerprint/dedup/base-canonicalization
- Fingerprint now uses `git status --porcelain -uall`, so editing a file inside
a brand-new untracked directory changes the fingerprint and the "Diff out of
date" banner fires (was collapsed to `?? dir/` and hashed as unreadable).
- extractTrackedPatchPaths pairs `---`/`+++` and excludes only the file's KEYED
path (new side, or old side for a pure deletion) — no longer drops a recreated
rename-source file (`git mv a b && touch a`). Verified rm --cached still
dedups to one entry and the recreated file still shows.
- resolveReviewBase canonicalizes a bare local default name ("main") to its
tracking ref ("origin/main") on every call, so a client that loaded before the
startup upgrade resolved can't revert the server to the stale local base on the
next refresh/switch. Both runtimes (new Pi resolveReviewBase helper).
- Removed the dead activeDiffLabel prop chain (FileTree -> DiffTypePicker).
- ADR recap: documented the staleness-banner scope limitation (default base only).
Verified: typecheck (all projects), bun test 23/23 review-core+fingerprint pass,
empirical repro of the untracked-dir fingerprint, rm --cached dedup, and
rename-recreate cases.
* fix(review): PR-990 review round 4 — dedup content, staging gate, banner timing
- Dedup rewrite: instead of dropping the untracked side of a same-path collision
(which hid content), drop the tracked DELETION block and keep the untracked
working-tree content. `git rm --cached f` + edit now shows the new content, not
a phantom deletion — still one entry per path (no dock/nav collision). New
stripHeaderPath also strips git's trailing-tab metadata, so space-named files
dedup correctly. Verified: rm --cached+modify shows content (1 entry),
space-named (1 entry), rename+recreate still shows both.
- All-files (and the `a` shortcut on the focused file) now gate staging per-file:
committed files in since-base mode are not stageable, matching SectionsPanel /
FileTreeNode. Shared isPathStageable helper threaded via ReviewStateContext →
AllFilesCodeView. Stops the confusing `git add` no-op that still flipped local
staged/viewed state.
- /api/diff/switch now awaits recomputeBaseBehindRemote before building the
response, so the "Baseline behind GitHub" banner reflects the new base
immediately (no ~5s lag switching in, no stale banner switching away). Both
runtimes.
- ReviewSetupDialog re-applies the recommended default only on first-run dismiss,
not when reopened from the header menu (was snapping a mid-session diff back).
- SectionsPanel "N added" header now counts sidecar-staged files too, so it
matches the staged dots on rows.
Verified: typecheck (all projects), bun test 1830 pass, empirical dedup repro,
both bundles build.
* refactor(review): reuse parsePatchPathToken in removeTrackedDeletions (self-review)
Drop the duplicated stripHeaderPath helper — the shared diff-paths
parsePatchPathToken already strips a/|b/ prefix + C-quoting + git trailing-tab,
and verifies the prefix instead of blindly slicing two chars. Documents the
binary-deletion edge (no --- line, so a binary rm --cached is not deduped).
* fix(review): PR-990 review round 5 — mixed-base + staging/sort/flicker nits
- Atomic base upgrade: the startup origin/* canonicalization swapped currentBase
without rebuilding the patch, so /api/diff could advertise origin/main while
the served hunks came from local main (mixed-base review on origin/HEAD-absent
repos). Now rebuilds the diff for the new base and commits base+patch+ref+
fingerprint together (only if no user switch happened); the fingerprint change
makes the client's freshness poll pick it up. Both runtimes.
- isPathStageable: gate staging OFF when since-base is active but the sidecar
hasn't loaded (was falling through to true, allowing a git-add no-op on a
committed file).
- SectionsPanel: session-staged files now float to the top of Changes — the sort
key was `false ?? stagedFiles.has(...)` which short-circuits; now ORs them.
- /api/diff/fresh: early returns now carry baseBehindRemote, so a snapshot change
mid-probe no longer clears the "behind GitHub" banner for one poll. Both runtimes.
- Removed the now-dead activeLabelFallback prop from DiffTypePicker (FileTree
stopped forwarding it in round 3).
Skipped (agreed): the fetch-base input-validation nit (no realistic attacker)
and the diff-switch TOCTOU (concurrent switches aren't UI-reachable).
Verified: typecheck (all projects), bun test 1830 pass, both bundles build.
* refactor(review): drop redundant staged-OR in SectionsRow (self-review)
item.staged (the grouping key) already ORs in stagedFiles as of the round-5
sort fix, so the row-level `|| stagedFiles.has(...)` is dead weight.
* fix(review): surface the startup base upgrade to already-loaded clients
Review round 6 fixes:
- The startup main -> origin/main upgrade re-baselined the freshness
fingerprint, so a client that fetched /api/diff before the rebuild kept
the old patch and every /api/diff/fresh probe reported fresh (the probe
compares server state to itself, never to what the client renders). Now
the fingerprint is only re-baselined when no client has loaded the
pre-upgrade snapshot; otherwise the stale baseline trips the normal
"Diff out of date - Refresh" banner. Bun + Pi.
- preserveFile refreshes (staleness Refresh, post-Fetch) now adopt the
server's returned base — exactly the paths where the server may have
canonicalized main -> origin/main; keeping the old name sent
/api/file-content and Ask AI context against the wrong base.
- ReviewSetupDialog: clamp the fixed 800px height to the viewport so the
dialog fits on small laptop screens.
- Docs: /api/diff/fresh response also carries baseBehindRemote/agentCwd.
- Documented the committed-deletion + untracked-recreation dedupe edge as
accepted (code comment + ADR) — fixing it needs two same-path diff
entries, which the path-keyed UI cannot represent.
* fix(review): stop the Git-status default from silently reverting to tree
Users with reviewPanelView=sections could keep opening in the tree view on
a stale diff type despite their cookie saying Git status. Three causes:
- The header Sections toggle persisted the view but not defaultDiffType,
creating a conflicted pair (sections + non-since-base default) that every
UI writer is supposed to prevent. It now couples the diff default like
the setup dialog and Settings do.
- configStore's debounced POST /api/config could be lost when a session
closed within 300ms of a change, leaving cookie and config.json split.
Pending writes now flush on pagehide with a keepalive fetch.
- configStore.init() applies config.json over the cookie without the UI
coupling, so a stale server value re-corrupted the pair on every load.
The app now self-heals at load: if the view says sections but the diff
default isn't since-base, it repairs the default (cookie + config.json)
and switches the live session to since-base.
* fix(ui): keep the stage (+) button border visible on the active file row
The button's --border border has no contrast against the active row's 30%
primary tint, so it vanished on the selected row until hovered. Tint the
border with the row's primary color on active rows (both sections and tree
views); the button's own hover border still applies.
* ui(review): spell out the panel view toggle — 'Git status | Tree' text instead of icons
* fix(review): PR-990 review round 7 — export label, diff snapshot race, unicode paths
- exportFeedback describeDiff(): add the missing since-base case — every
feedback export in the new default mode read "**Diff:** since-base".
- /api/diff GET: snapshot patch/base/ref/error BEFORE the sections-sidecar
await and pass the pinned base into buildSectionsSidecar. The startup base
upgrade landing mid-await could pair a rebuilt patch with sections grouped
against the old base, with initialDiffServed still false so no refresh
banner ever came. Bun + Pi.
- SectionsPanel: a session-staged untracked file now moves to the Changes
section immediately (anticipating the server's next sidecar) instead of
sitting in Untracked with a staged dot until refresh.
- unquoteGitPath: real C-style unquoting with octal (UTF-8 byte) escape
decoding. JSON.parse rejects octal escapes, so non-ASCII names kept their
literal \303\251 form — an untracked "café.txt" was silently ABSENT from
the review (git diff --no-index could not access the quoted name) and the
deletion dedupe Set lookup could never match. getUntrackedFileDiffs now
unquotes ls-files output. Unit tests: octal decoding + end-to-end unicode
untracked file in a real repo.
- review-core.test: worktree subtype round-trip now covers since-base and
all (was 6 of 8).
Parked (deliberate): SectionsPanel/FileTree keyboard-nav dedup refactor.
* fix(shared): unquoteGitPath keeps literal unicode intact (self-review)
The byte-collector treated literal non-ASCII code units as single bytes,
which would mojibake headers synthesized by our own quoteGitPath
(JSON.stringify leaves unicode unescaped inside quotes — workspace-mode
prefixed headers round-trip through the same parser). Literal chars now
append as string code units (surrogate-safe); only octal escapes go
through the UTF-8 byte decoder.
* fix(review): PR-990 review round 8 — pre-staged toggles, explicit base, AI context race
- useGitAdd: session Set replaced with a tri-state override map folded over
the sections sidecar; stagedFiles is now the EFFECTIVE staged set (sidecar
+ session stages - session unstages). Fixes pre-staged files: the first
`a` press actually unstages (was a git-add no-op), the sidebar dot clears
after a real unstage (was stuck via sidecar OR), and the All-files header
agrees with the sidebar on load. Overrides reset whenever a fresh sidecar
arrives (switch, preserveFile refresh, PR response) so stale session
intent can't fight new porcelain truth. SectionsPanel drops its own
sidecar OR; stagedCount = effective size.
- Explicit base picks are honored verbatim: the picker sends explicitBase,
and the server permanently disables local-name -> origin/* canonicalization
once set (the local/remote groups are distinct choices). The behind-GitHub
banner also exempts an explicitly-picked local name — Fetch advances
origin/*, so the banner would be un-clearable nagging. Bun + Pi.
- buildCurrentAiReviewContext(patch, base): GET /api/diff builds Ask AI
context from the same served snapshot as the patch — the startup base
upgrade could hand Ask AI a different changeset than the screen. Bun + Pi.
- recomputeBaseBehindRemote: capture remoteDefaultInfo once — a concurrent
refresh nulling it mid-await threw. Bun + Pi.
- Revert stray "Status Update / Updated!" edit to adr/0001 (test debris
swept into the round-2 commit).
- splitPorcelainRename comment corrected (porcelain v1 does NOT quote plain
spaces; a name containing " -> " is ambiguous without -z) + ADR notes for
the index-only-changes semantics and the rename edge.
* fix(review): FileTreeNode uses the effective staged set — round 9
Round 8 made stagedFiles the effective set (sidecar + session overrides)
and removed SectionsPanel's sidecar OR, but missed the same OR in
FileTreeNode's since-base row (sectionStaged). In the Tree fallback a
pre-staged file unstaged this session kept its dot and the next toggle
re-staged it. Grep-swept: this was the last surviving sidecar-staged OR.
* refactor(review): make the staged-display invariant unrepresentable
The round-8/9 bug class (sidecar staged flag ORed over the effective set)
existed because surfaces had a second staging source to reach for. Remove
it: stagedFiles is now a REQUIRED prop on SectionsPanel/FileTree/
FileTreeNode and the optional-prop fallback branches reading
sectionEntry.staged for display are deleted — a future surface cannot
reintroduce the OR because the pattern no longer exists to copy. The
sidecar type's staged field and AGENTS.md now document the invariant at
the point of temptation. Grep for display reads of .staged now hits only
useGitAdd (owner) and the sidecar builder (producer).
* refactor(review): self-review cleanups on the staged-invariant hardening
- orderFilesBySections: document why its snapshot .staged read is safe
(only called at sidecar-fresh moments where snapshot = effective) and
that it must not be reused mid-session — the one remaining display-side
snapshot read the invariant sweep surfaced.
- FileTreeNode: drop the now-pure sectionStaged alias; use isStaged.
* fix(review): PR-990 review round 10 — header staging gate, fingerprint cap, single-writer coupling
- Single-file diff header now uses the per-path staging gate (canStagePath),
closing the last ungated staging trigger: committed-only files in
since-base offered a no-op Git Add that flipped local staged/viewed state.
Full trigger inventory swept: App `a` shortcut, all-files `a` + header,
SectionsPanel rows, FileTreeNode rows, single-file header — all six now
per-path gated or group-gated; no context-menu staging exists.
- Fingerprint circuit-breaker: the freshness poll's `git status --porcelain
-uall` degrades permanently (per cwd, per process) to collapsed -unormal
once its output exceeds 2MB — a forgotten node_modules/ no longer burns
CPU every 5s for the whole session. Costs untracked-dir edit sensitivity
only on such repos; one possibly-spurious staleness banner at the switch.
- The sections ⟺ since-base coupling now has a single writer:
setReviewPanelView/setReviewDefaultDiffType in @plannotator/ui/config.
All five call sites (setup dialog x2, Settings x2, header toggle,
first-run reset, self-heal) converted; grep for direct writes of either
setting now hits only reviewView.ts.
- /api/diff/switch responses pass snapshot args to the AI context builder
(both branches, Bun + Pi) — correct today, now robust against future
awaits between the epoch check and the response.
- Docs: explicitBase in the /api/diff/switch body.
Parked per discussion: SectionsPanel/FileTree nav+search+footer dedup
(extract when the commit-list view adds a third panel), querySelector row
measurement, fetch-base stderr passthrough (standing decision), Pi
hasAgentLocalAccess (pre-existing follow-up list).
* fix(review): PR-990 review round 11 — subdirectory launches, escape decode, settings note
- Repo-root-relative patch paths now resolve against the git toplevel
everywhere they meet a filesystem path or pathspec, via a shared
resolveRepoToplevel helper: file-content working-tree reads (hunk
expansion returned null from a subdirectory launch) and gitAddFile/
gitResetFile (stage/unstage failed with pathspec errors). Both bugs
pre-existed for uncommitted/unstaged; since-base made them the default
experience. The two existing inline toplevel resolutions (untracked
diffs, fingerprint) now use the same helper. Shared code — Pi inherits
via vendoring. Real-repo subdirectory tests for both.
- unquoteGitPath decodes \uXXXX (JSON.stringify emits it for control
chars without a short escape; our synthesized workspace headers
round-trip through this decoder). Malformed \u stays literal. Tests.
- Pi explicitBase guard matches Bun byte-for-byte on empty-string base.
- Settings Git tab notes when the CURRENT repo can't serve the Git-status
view (base ref unresolvable) instead of letting the preference look
silently broken — it's a global preference, so the options stay.
Same-class items left parked (pre-existing, untouched by this PR):
open-in root resolution and code-nav file reads from subdirectory
launches — on the follow-up list with the Pi divergences.
* fix(review): PR-990 review round 12 — per-client freshness, keyboard-operable row controls
- Freshness is now judged PER CLIENT: every patch-carrying response
(/api/diff, /api/diff/switch, pr-switch, pr-diff-scope) includes
snapshotId (the server's draftKey), and the client echoes it on
/api/diff/fresh probes. A mismatch reports stale for THAT client
regardless of the VCS fingerprint. This fixes the round-12 finding —
reloads/second tabs after the startup base upgrade got a permanently
bogus staleness banner from the shared pre-upgrade baseline — and
deletes the round-6 conditional re-baseline hack entirely (the
fingerprint recaptures unconditionally again; the old client's banner
now comes from its snapshot mismatch, not a deliberately stale
baseline). Also gives unfingerprintable modes (P4, PR layer) snapshot-
level staleness for free. Bun + Pi + client hook.
- StageControl and ViewedControl (which had the identical gap) are
keyboard-operable: tabIndex + Enter/Space activation + focus outline.
They're spans inside the row <button> (nested real buttons are invalid
HTML), so they need their own focus stop; the a/v shortcuts remain the
power path.
- Docs: snapshotId on /api/diff, ?snapshot= on /api/diff/fresh.
* fix(review): PR-990 review round 13 — re-key snapshots on scope switch, PR-tab refresh
- The PR scope switch now re-keys draftKey (= snapshotId + draft storage
key) at BOTH commit points, unconditionally — matching every other
snapshot commit site. The full-stack branch previously kept the layer
patch's key: stale layer tabs never got the banner after a cross-tab
scope switch (the exact case snapshotId exists for), and full-stack
drafts collided with layer drafts (pre-existing). The layer branch's
!layerPatchIncomplete conditional is gone — it only stayed consistent
because full-stack never re-keyed. Invariant now: every currentPatch
commit is followed by a re-key. Bun + Pi.
- Refresh works for any stale PR tab: re-selects the CURRENT scope
instead of no-opping for layer (only full-stack could go stale in the
fingerprint-only world; snapshot mismatch changed that). Accepted
residual (documented in code): after a cross-tab PR switch, refresh
updates the patch but not prMetadata — the scope endpoint doesn't
carry it, and the full-rehydrate refactor isn't worth the two-tab edge.
* fix(review): stale incomplete-layer Refresh uses the non-blocking upgrade path (self-review)
Round 13 made Refresh re-select the current scope for stale PR tabs; for
an INCOMPLETE layer patch that POST triggers the server's local recompute,
which can park for minutes behind checkout warmup — and
handlePRDiffScopeSelect renders the full-screen switch overlay the whole
time. Route that case through handleLoadFullDiff (same POST, progress
notice instead of modal), which exists for exactly this slow path.
* fix(review): round 14 — composite snapshot id, fully-pinned /api/diff, "All changes" label
- snapshotId is now content hash + diff type (+ PR scope), built by a
single currentSnapshotId() helper used at every response site and the
freshness compare. A cross-tab MODE switch with a byte-identical patch
(layer vs full-stack on a single-PR stack) now flags old tabs; the base
is deliberately excluded so a same-commit main -> origin/main
canonicalization stays banner-silent (round-12 noise-avoidance kept).
draftKey stays a pure content hash — drafts survive content-identical
round-trips. Bun + Pi.
- GET /api/diff pins ALL served fields (diffType, hideWhitespace,
prDiffScope join patch/base/ref/error/snapshotId) and the sidecar + AI
context builders take the pinned type instead of reading globals — a
concurrent tab switch during the sidecar await can no longer produce a
since-base patch labeled with another mode. Bun + Pi.
- First-user feedback: the since-base label is now plain English —
"All changes since origin/main" (dynamic, follows the picked base);
dialog/Settings short form "All changes"; feedback exports match.
uncommitted reverts to "Uncommitted" where it had borrowed "All
changes", so the two stay distinguishable side by side.
* docs(review): finish the Since-main -> All-changes terminology sweep in comments (self-review)
* fix(review): PR-990 review round 15 — base-revert race, freshness reset, unstage regroup
- resolveReviewBase gains a second, probe-independent rule: a non-explicit
echo of the bare local name of the CURRENT origin/* base stays on the
tracking ref. The existing rule keys off remoteDefaultInfo, which comes
from a second network probe that can lag the startup upgrade by seconds;
in that window a diff-type/whitespace switch echoing "main" committed
the session back onto the stale local branch and set baseEverSwitched,
permanently blocking the upgrade. Bun + Pi.
- useDiffFreshness resets stale/dismissed state on snapshotId too — since
round 14 a new snapshot can reuse identical patch text with a different
id, and the old banner state wrongly carried over until the next poll.
- SectionsPanel: unstaging a PRE-staged add moves it back to Untracked
(mirror of the round-7 stage regroup). Detection deliberately uses the
sidecar's snapshot flag to recognize "was pre-staged"; staged
modifications stay in Changes, staged renames remain a refresh-heals
edge.
* fix(review): PR-990 review round 16 — rename staged count, guarded fetch-base replay
- "N added" no longer double-counts staged renames: the sidecar truthfully
marks BOTH porcelain sides staged, but an above-threshold rename renders
as ONE file — the hidden old path inflated the count and left a phantom
effective-staged entry that unstaging the visible row couldn't clear.
The client now filters sidecar-staged paths to rendered files, which is
correct in both patch shapes (below-threshold renames render delete+add
as two rows and both sides pass the filter). Client-only; the sidecar
stays faithful to porcelain.
- The fetch-base completion only replays the diff refresh if the user's
diff type/base selection is unchanged since the click — a slow fetch no
longer yanks the review back to the view captured at click time.
Declined with reasons (in review thread): first-run persist-before-confirm
(approved forced default, second flagging), explicit-base flag ordering
(early-set preserves user intent; commit-after-win would leave picker and
server disagreeing), base-picker revert target (near-unreachable compound
race, deferred).
* fix(review): PR-990 review round 17 — find origin/main on feature-only clones
getDefaultBranch's chain (origin/HEAD -> local main -> blind "master")
skipped the fetched remote-tracking ref entirely. On checkouts with no
origin/HEAD symref and no local main/master — CI checkouts, `clone
--branch feature`, extra worktrees — it guessed "master", the base
didn't resolve, getGitContext suppressed since-base, and the flagship
"All changes" view silently disabled itself for the whole session even
though origin/main was fetched and diffable. The startup upgrade can't
rescue it either (its guard reads the bogus "master" as a deliberate
non-default base).
Chain is now: origin/HEAD (verified) -> origin/main -> local main ->
origin/master -> "master" — remote-tracking refs preferred, matching the
function's stated prefer-upstream intent. Shared core (Pi inherits);
real-repo test reproducing the exact clone shape.
|
||
|
Michael Ramos
|
e3de938914 |
feat(review): PR Overview panel + description/comment annotations + media (#981)
Combine the PR Summary/Comments/Checks tabs into one PR Overview panel, then make the description and comments annotatable and render their media. - PR Overview panel (one sidebar entry) + comment UI (avatars, filters, hide bots, live context, responsive stacking). - Annotate the PR description (select → comment) and PR comments (Annotate button), with Ask AI; notes show in the Annotations sidebar and ship to the agent. - Split/Unified diff toggle relocated into the dock tab strip. - Render images + video in descriptions and comments (raw HTML + markdown), capped to the card so nothing bleeds. - Review-flow fixes: copy-all feedback, prose-only feedback preamble, no image control on prose notes, GitHub review-body seeding; stronger review trailer. - Add Claude Sonnet 5 as the default Ask AI model. No server, endpoint, or Pi-runtime changes. |
||
|
Michael Ramos
|
740d6fb2eb |
Add WebTUI agent panel to annotate mode (#941)
* feat(annotate): add WebTUI agent terminal * feat(annotate): wire WebTUI agent into annotate UI * docs: recap annotate agent terminal work * fix(annotate): harden agent terminal runtime * docs: add annotate agent terminal runtime ADRs * fix(annotate): polish agent terminal integration * fix(ui): preserve comment draft on Ask AI failure * fix(annotate): address terminal review findings * fix(annotate): harden agent terminal runtime fallback |
||
|
Michael Ramos
|
2a6fe5c457 |
Open files in external apps + code-review UX pass (#942)
Adds a server-side "open the current file in an external app" control to the code-review and annotate surfaces (split-button with host-detected apps; last-used becomes the default; cross-platform launch mirrored across the Bun and Pi runtimes), plus a code-review UX pass: file-header change letters and line counts, the diff-settings cog grouped with Split/Unified, an all-files collapse/expand-all toggle, and the semantic diff moved to a resizable sidebar accordion. Also fixes Cmd+click code navigation (worker token transformer). Hardened over three adversarial review rounds: shared, tested path-containment for open-in (Bun + Pi), annotate scoping aligned to /api/doc reference roots, strict PR-checkout rooting (never the launch repo), deduped app-catalog and semantic-diff fetches, macOS bundle-only availability, Windows launch fixes (reveal + terminal off cmd's parser), and PR-checkout tracking across switch and pool warmup. |
||
|
Michael Ramos
|
195328f7a6 |
Persist saved annotate file edits in drafts (#936)
* Persist saved annotate file edits in drafts * Handle stale saved file edit context * Test source edit conflict actions * Return source metadata for single-file docs * Fix saved file edit conflict races * Handle deleted source files in annotate edits * Tighten annotate missing-file recovery * Reset edit state for missing file reopen * Tighten source edit restore path handling * Harden source edit recovery paths * Harden source edit disk reconciliation * Fix live file tree startup delay * Document file tree watcher startup ordering * Preserve source save through missing files and symlinks * Harden missing source file recovery * Tighten annotate source edit boundaries |