feat(document-viewer): PDF-first citation landing with condensed overview - #1705
Conversation
…view Stop deep-linked citations from auto-scrolling into the indexed source-passages dump. Keep a short cited-excerpt chip under the PDF, open indexed text only on inspect/search/section jump, and tighten phone overview actions so the first viewport reaches the source sooner.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 44 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe document viewer now lands citations on the PDF preview. Indexed text opens only after explicit inspection or search. The cited excerpt is compact, the rail removes duplicate evidence rendering, and mobile overview actions use a collapsible layout. ChangesCitation viewer behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DocumentViewer
participant PDFPreview
participant PinnedSourceEvidence
participant IndexedTextPanel
DocumentViewer->>PDFPreview: land citation on PDF preview
DocumentViewer->>PinnedSourceEvidence: render compact cited excerpt
PinnedSourceEvidence->>DocumentViewer: request indexed-text inspection
DocumentViewer->>IndexedTextPanel: send reveal request
IndexedTextPanel->>IndexedTextPanel: reveal and navigate to cited chunk
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR updates the Document Viewer’s citation deep-link landing behavior to be PDF-first, showing a condensed Cited excerpt surface and keeping the indexed source-text dump collapsed unless the user explicitly opts in (inspect/search/section nav). It also tightens the phone overview action chrome and removes duplicate cited-excerpt UI from the desktop rail.
Changes:
- Citation deep-links now scroll to the PDF preview section instead of auto-scrolling into indexed source text, and add an explicit Inspect indexed text affordance.
- Renames/reshapes the “Pinned evidence” section to Cited excerpt and removes the duplicate rail card so there’s a single excerpt surface.
- Updates DOM + Playwright smoke coverage to assert the new collapsed-by-default behavior and the new section label.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/ui-smoke.spec.ts | Updates smoke assertions for collapsed indexed-text on citation landing + “Inspect indexed text” flow + section label rename. |
| tests/document-section-summary.dom.test.tsx | Adds DOM coverage for citation landing staying collapsed unless explicitly revealed. |
| tests/document-section-nav.dom.test.tsx | Updates section index expectations from “Pinned evidence” to “Cited excerpt”. |
| src/components/DocumentViewer.tsx | Changes citation landing scroll target to the PDF section; wires “Inspect indexed text” to an explicit reveal request. |
| src/components/document-viewer/source-panels.tsx | Implements “Cited excerpt” panel updates and adds revealRequest to control IndexedTextPanel opening rules. |
| src/components/document-viewer/section-index.ts | Renames the section label to “Cited excerpt”. |
| src/components/document-viewer/document-rail-panels.tsx | Removes the duplicate cited-excerpt panel from the rail. |
| src/components/document-viewer/document-overview-landing.tsx | Condenses phone overview chrome and moves secondary actions behind “More actions”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/document-section-summary.dom.test.tsx (1)
76-111: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear the inspect reveal state after opening
inspectIndexedTextSectionsetsinspectRevealKeybut never clears it. For the same citation,forceRevealremains true, so the controlled panel reopens after every close and renders a non-interactive summary. Make the inspect reveal one-shot, then update the test contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/document-section-summary.dom.test.tsx` around lines 76 - 111, Update inspectIndexedTextSection to clear inspectRevealKey after the requested citation opens, making forceReveal a one-shot state so the panel can close normally and its summary becomes interactive. Revise the test “opens on an explicit inspect request and keeps the cited chunk revealed” to assert the reveal state is consumed and subsequent closing does not reopen the panel.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/branch-review-ledger.md`:
- Line 714: Update the 2026-08-07 ledger entry for
cursor/document-citation-landing-7bc3 to include the decisive verification gate
output line proving the relevant gates passed, rather than only the summaries
“typecheck,” “lint,” and “verify:pr-local stages green.” Preserve the existing
first-run design-system-adoption timeout and retry-success note.
In `@src/components/DocumentViewer.tsx`:
- Around line 167-171: Reset IndexedTextPanel’s local disclosure state whenever
citationLandingKey changes without an active inspect reveal, so navigation from
citation A to B restores PDF-first behavior instead of leaving compactOpen
enabled. Use the existing citationLandingKey to trigger the reset or remount,
and add a regression test covering inspect A followed by navigation to B.
---
Outside diff comments:
In `@tests/document-section-summary.dom.test.tsx`:
- Around line 76-111: Update inspectIndexedTextSection to clear inspectRevealKey
after the requested citation opens, making forceReveal a one-shot state so the
panel can close normally and its summary becomes interactive. Revise the test
“opens on an explicit inspect request and keeps the cited chunk revealed” to
assert the reveal state is consumed and subsequent closing does not reopen the
panel.
🪄 Autofix
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a0527948-5a67-443e-85d1-bec24716d4a9
📒 Files selected for processing (9)
docs/branch-review-ledger.mdsrc/components/DocumentViewer.tsxsrc/components/document-viewer/document-overview-landing.tsxsrc/components/document-viewer/document-rail-panels.tsxsrc/components/document-viewer/section-index.tssrc/components/document-viewer/source-panels.tsxtests/document-section-nav.dom.test.tsxtests/document-section-summary.dom.test.tsxtests/ui-smoke.spec.ts
|
An unexpected error occurred while generating fixes: Request timed out after 1200000ms (requestId=501683cc-ab44-41f8-936f-ff37d5662bf1) |
Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com>
…n navigation
Fixes the CI failure in ui-smoke ("document viewer content disclosures
are naturally closed and mutually exclusive by default"): jumpToSection
only set inspectRevealKey when navigating TO "source-text" but never
cleared it navigating away. Since IndexedTextPanel's `open` is a
React-controlled prop bound to that key, it stayed forced open on every
re-render (including the one selectSection triggers) and, sharing the
native exclusive `name="document-viewer-section"` accordion group with
the section just jumped to, silently closed it right back.
Same root cause independently flagged by the Sentry and CodeRabbit
review threads on this PR (inspectRevealKey never reset on navigation).
|
@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker. If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop. Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them. Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head. Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval. Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1. Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me. |
|
❌ Unable to add follow-up after multiple attempts. The agent may be experiencing issues. Please try again later or Open in Web to view the status.   Learn more about Cursor Agents |
|
@cursoragent fix merge conflict |
|
Agent is starting up. I'll add your follow-up shortly. |
|
@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker. If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop. Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them. Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head. Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval. Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1. Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me. |
|
❌ Unable to add follow-up after multiple attempts. The agent may be experiencing issues. Please try again later or Open in Web to view the status. Learn more about Cursor Agents |
|
@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker. If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop. Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them. Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head. Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval. Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1. Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me. |
Resolve real conflicts in outstanding-issues (prefer main queues), design-system COMPONENTS (regenerated adoption), and document-rail-panels (keep filmstrip from main; drop unused ChunkRow after citation evidence moved out of the rail). Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…l clear Close IndexedTextPanel when revealRequest falls on the same citation so exclusive accordion jumps away from source-text stay open, and clear inspectRevealKey whenever the citation landing key changes so revisiting a prior citation stays PDF-first. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Behind-but-clean against origin/main; clear GitHub mergeability DIRTY so required CI can build the merge ref. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
PR #1705 end-to-end summaryTip: Sync
Review / BugbotPrior Sentry/Copilot/CodeRabbit threads on
No protected RAG/ranking surfaces edited. No new unresolved review threads. Required CI (this tip)Green so far: Change scope, PR policy, PR mergeability, Static PR, Safety/config, Build, Semgrep (+ ingestion), Gitleaks, GitGuardian. Local gates (decisive)
Not run (no separate approval): Residual riskDual state machine ( Merge left to you. Ledger row for this head appended locally under |
…provenance contract (#1729) * test(visual): adopt the six linux visual baselines from CI run 31251091603 `tests/__screenshots__/` has held only a README since the visual gate landed, so every target sat in `AWAITING_BASELINE`, skipped rather than compared, and no PR could prove it had not moved the look. That is the blocker `#118` records and the one `#269` is formally waiting on. Adopted from the ubuntu CI artifact `visual-baseline-31251091603` — a `push` on `main` at `bc33d414e`, never from a developer machine: `snapshotPathTemplate` is platform-scoped, so win32 PNGs land in `win32/` and the ubuntu job never reads them, and font hinting alone would make every later run red. All six goldens were reviewed before committing, and stability was measured rather than assumed: five of the six are byte-identical by SHA-256 to the candidates from the earlier independent run 31249978408 at `2069b1f5c`. Only `document-viewer` differs, and those two runs straddle #1705, which changed `DocumentViewer.tsx`. `AWAITING_BASELINE` is now empty, so a missing golden is once again unambiguously a fault rather than a declared exemption. The two honesty tests keep that list from rotting in either direction. Deliberately NOT included: promotion. `visual-baseline` stays `continue-on-error` and outside `pr-required`. The workflow comment asks for a soak first, and adding the job to `pr-required` and dropping `continue-on-error` belong in one later edit. Known limitation recorded in the README rather than papered over: the `document-viewer` target clips a ~2900px `#main-content` against a 900px viewport and contains viewport-pinned chrome (`sm:sticky` header, `sm:fixed` composer), which composites partway down the stitched capture and overlaps content. It is deterministic, so the comparison still means something, but it predates #1705 and would be quieter with a narrower clip or a mask. Refs #118 * test(visual): satisfy the committed baseline provenance contract The first commit adopted the six PNGs but missed `tests/design-system-adoption.test.ts`, a committed contract that governs exactly this adoption and that no handover in this series mentions. It was dormant only because it had no committed baselines to check. `npm run verify:cheap` caught it: 9 failed / 5586 passed. Three things it requires, all now met: - `tests/__screenshots__/linux/provenance.json` (schemaVersion 2) binding the goldens to their capture: platform `linux`, runner `ubuntu-24.04`, hosted-CI artifact `visual-baseline-31251091603`, candidateSourceHead `bc33d414e`, and a SHA-256 plus pixel dimensions per candidate. The reviewer attestation is a human one — recorded after the six PNGs were actually looked at, not asserted on their behalf. - `AWAITING_BASELINE` must stay a *static literal* `new Set([...])`, and the suite may differ from the capture commit in nothing but those values. The explanatory comment added in the first commit violated that, and `new Set<string>()` did not parse as a literal set at all. Both reverted; the suite diff is now exactly the six ids. - Only an allowlisted set of paths may change alongside the adoption, which is why the `tests/__screenshots__/README.md` note from the first commit is reverted here. The document-viewer limitation it recorded is in the PR description instead. The fixture change is the one substantive fix. `initialiseCandidateRepository` seeded its candidate-source commit from the *live* `tests/ui-visual-baseline.spec.ts`, so every fixture silently depended on the repository not having adopted its baselines yet — and the commit that empties `AWAITING_BASELINE`, the outcome the contract exists to permit, made the fixture's own candidate head fail "must contain exactly the canonical six ids". It now defaults to the explicit canonical list. A fixture states its precondition rather than borrowing it from the tree under test. Verified: tests/design-system-adoption.test.ts 51 passed (51). Mutation-tested by corrupting a provenance SHA-256, which fails with the expected mismatch. Refs #118
…view (#1705) * feat(document-viewer): PDF-first citation landing with condensed overview Stop deep-linked citations from auto-scrolling into the indexed source-passages dump. Keep a short cited-excerpt chip under the PDF, open indexed text only on inspect/search/section jump, and tighten phone overview actions so the first viewport reaches the source sooner. * docs(ledger): record document citation-landing review for PR #1705 * Reset compactOpen when citation changes without an active reveal request Co-authored-by: BigSimmo <87357024+BigSimmo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
…lready require jsdom@30 requires Node ^24.15.0, but engines.node said "24.x" and every gate compared majors only. A container on 24.13.0 therefore passed check:runtime and the preinstall hook, then failed `npm ci` with an opaque EBADENGINE for a transitive package. That blocked local verification on PRs #1611, #1697, #1705 and #1740, each worked around by hand. - package.json: engines.node "24.x" -> ">=24.15.0 <25", declaring the floor the dependency tree already imposes. This cannot newly break any environment that currently installs, since jsdom enforces the same floor during `npm ci`. - check-node-engine.cjs (npm preinstall): compare the full version against that range instead of the major, so the failure names the real requirement. Kept import-free — the Dockerfile COPYs this file alone before `npm ci` — with the range restated as a literal and pinned to package.json by test. - check-runtime.ts: add NODE_MINIMUM_VERSION and reject a matching major below it. This gate fronts verify:cheap, :ui, :release and :pr-local. - session-start.sh: provision 24.19.0, and make the install guard floor-aware. The old major-only test meant a cached container already on 24.13 skipped the install entirely, so bumping the pin alone would not have upgraded it. - check-codex-cloud-setup.mjs: accept the range form, still requiring its major to track .node-version. Closes #285. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XZHZjz6kX8Ly1qBvvCg29C
…e claiming "0 commits ahead" when it isn't (#2117) * fix(claude): make the session-start hook runnable and give the agent config teeth `.claude/hooks/session-start.sh` was committed as mode 100644 while both its siblings were 100755, and it is the only hook registered by bare path rather than through `bash`. Its whole body is gated on CLAUDE_CODE_REMOTE=true, so the one environment it does any work in is a Linux web container — exactly where a non-executable checkout cannot run. It is also the script that provisions the Node 24 the engine floor requires, after npm ci EBADENGINE blocked PRs #1611, #1697, #1705 and #1740. The defect was invisible locally: the primary workstation is a Windows ReFS Dev Drive with core.fileMode=false, so git ignores filesystem permission bits and a local `chmod +x` is a silent no-op. Only `git update-index --chmod=+x` can fix it. Fixed three ways so it cannot recur: the index mode, a `bash "..."` registration that stops the mode being load-bearing, and a contract test. Also in this change: - .claude/settings.json gains a permissions block. AGENTS.md's provider confirmation boundary was prose-only; this encodes it as deny/ask rules, including deny on reading .env* (a staging key leaked on 2026-08-18) and on the Supabase MCP write tools. The repo already learned that prose does not hold here — see the comment in pr-handoff-stop.sh. - check-base-freshness.mjs now emits its stale-base warning on stdout as hook JSON. Every human-readable branch used console.error, and Claude Code injects only stdout into context, so the tripwire never reached the agent. The origin/main fetch also gains a 10s timeout so a hung remote cannot burn the whole SessionStart budget. - clean-worktree.mjs gains list-only `--merged` and `--squashed`. Nothing reclaimed merged worktrees, so 49 accumulated, ~19 GB of duplicated node_modules on a 50 GB Dev Drive. Ancestor detection alone finds 2 of 49 because this repo squash-merges; the patch-id test finds 9. A `confidence:` line distinguishes proven from inferred, because the two are not the same claim and one candidate had 2 of 21 files still differing. - Explicit hook timeouts, a PreCompact hook that asks for /issues capture while the context still exists, and a push format guard that only fires where the .githooks pre-push guard is not wired. Removal stays a separate opt-in throughout; `runWorktreeCleanup()` is byte identical, so verify:preflight is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): stop the session-start hook test failing on every Windows run `tests/session-start-hook.test.ts` asserted that the written env file contains `join(home, ".node24", …)`. On Windows `home` comes from mkdtempSync(tmpdir()) as `C:\Users\…\AppData\Local\Temp\session-start-home-XXXX`, while the hook runs under Git Bash and writes the POSIX view of the same directory, `/tmp/ session-start-home-XXXX`. The assertion therefore failed on every Windows run regardless of the diff under test. That is worse than a red test: it is a red test everyone learns to ignore. It fails inside `npm run test`, which is the last step of `verify:pr-local`, so the whole gate goes red locally for every change and the only way to use it is to decide which failures do not count. Compare the path tail instead. The unique mkdtemp basename still pins the assertion to this test's own HOME, so it loses no strength, and it now holds on both platforms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the Claude Code environment review for PR #2113 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(claude): pin the permission boundary and make the PreCompact hook answerable Closes the four items left open when PR #2113 was opened, each with the smallest fix that actually resolves it rather than restating it. 1. The claim that no `allow` rule can reach a provider-backed script was asserted in review and never measured. It is now a test: 36 provider-backed scripts, derived from script names rather than hand-listed, each asserted unreachable through `allow` and covered by an explicit `ask`. Mutation-checked — injecting a broad `Bash(npm run check:*)` allow rule turns 4 of them red. 2. Hook registrations are pinned to invoke through an interpreter rather than a bare path, and to carry an explicit timeout. Bare-path registration is what made the `session-start.sh` mode bug reachable; the mutation check confirms both guards bite. 3. The PreCompact hook's known limit could not be closed by reading code — the installed CLI ships a compiled binary with no inspectable bundle, so whether the platform injects its stdout into model context is not determinable here. Instead of leaving that permanently unverified, the hook now appends one line per firing to a log under the git dir. After the next compaction the log distinguishes "hook never ran" from "hook ran but its output went nowhere", and both answers are actionable. The log lives outside the worktree so it can never be staged. 4. The `newtask` skill said "~40 worktrees". It was 48 on 2026-08-18 and reached 50 during one session, so the number now carries the reason it drifts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(issues): queue the five follow-ups this session could not close Each of these is blocked on something outside the repo, so capturing them is the smallest fix that resolves them — the alternative is that they die with the session context. - P1 rec: PR churn has exhausted both review bots' budgets, so PR #2113 landed with zero automated review and subsequent PRs will too. AGENTS.md already measured the CI half of this cost; this is the second bill and the worse one. Needs a decision on whether the bundling rule gets a gate. - P2 task: nine landed worktrees, ~4.5 GB on a 51%-full Dev Drive, deliberately not removed — re-verification immediately before deletion showed two held unmerged commits despite the scan reporting none minutes earlier, and a third had been switched branches mid-scan by a live session. - P3 task: confirm D:\.npm-cache is a registered Dev Drive trusted cache; fsutil needs elevation and the non-elevated registry fallback reads empty. - P3 task: read the PreCompact hook's own log after the next compaction to settle whether its output reaches model context. The log lives under the worktree's git dir, so check it before cleaning that worktree up. - P3 task: confirm on a real web session that session-start.sh now runs. The mode bug was proven; the failure it would cause on Linux was not, because no container was available. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(clean-worktree): stop the candidate listing claiming "0 commits ahead" when it isn't Every `--squashed` candidate printed "0 commits ahead" while being genuinely ahead of origin/main — by 11, 2, 1 commits on the real fleet. A squash-merged branch keeps its original commits forever, so it stays ahead permanently; what is zero is the count of UNLANDED commits, which is a different claim. The line was therefore stating something a reader could disprove with one `git rev-list`, on a tool whose entire job is to be trusted before a deletion. It now reports both numbers: "11 ahead of origin/main, 0 unlanded commits". Also corrects the comment above the ahead check, which described it as belt-and-braces against the merge test. That is true in ancestor mode, where it is the real gate. In squash mode it is not a second opinion at all — gitAheadUnlandedCount returns 0 for any branch the squash test just accepted, so the check is satisfied by construction and can only fire on a candidate that was already skipped. Keeping it is correct; describing it as independent evidence was not. The raw count is reporting only and never gates, so no candidate set changes. Verified against the live fleet: same 9 candidates before and after, worktree count unchanged, --remove not run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the follow-up review for PR #2117 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Summary
origin/mainto clear real merge conflicts (docs/outstanding-issues.md,docs/design-system/COMPONENTS.md,document-rail-panels.tsx).compactOpenwhenrevealRequestfalls on the same citation, and clearinspectRevealKeywhen the citation landing key changes so revisits stay PDF-first.Test plan
tests/document-section-summary.dom.test.tsx+document-section-nav.dom.test.tsx(17 passed)npm run verify:cheapnpm run verify:pr-localRAG impact: no retrieval behaviour change — DocumentViewer citation UI landing only; no rag/, ranking, or retrieval RPC edits.
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes