Name which value holds the phone header open, not just that it did - #1430
Merged
Conversation
…that it did `data-scroll-hidden` on the collapse wrapper is `scrollHidden && !sharedChromePinned`, so a missing attribute has two very different causes the assertion cannot separate: the scroll state machine never fired, or it fired and a pin held the chrome open. The bare assertion reads as the first even when it is the second — which is how a stuck pin was misread as a flaky scroll gesture across two CI runs on 2026-07-30. `expectChromeHidden` keeps the same pass condition and adds a failure message. The discriminator is already in the DOM: DocumentViewer's page-owned composer hides on `composerScrollHidden`, which consults `scrollHidden` and not the pin, so composer-hidden plus header-visible proves the pin. Every term of `sharedChromePinned` also has a DOM tell — an `aria-expanded` trigger, a popover, or focus inside the portaled addon host — so when all read false the pin is a stale latch rather than a live surface, and the message says so. Also updates ledger #127 with what the traces establish: `scrollHidden` is TRUE and `sharedChromePinned` is stuck, reproducible on both completed full-suite runs and both variants, always at the reduced-motion hide that follows the section-sheet round-trip and never at the first hide. `main` only looks green because `Production UI` is skipped on its docs-only pushes; it has not run this test since 90b3e34, with zero `src/` changes since. Deliberately not the fix. Which term latched is proven; the mechanism is inferred, and it does not reproduce locally — every local run used the container's Chromium 1194 rather than the bundled 1234 CI installs (#121), so no local green is evidence here. This makes the next CI failure name its own cause instead of costing another trace download. Verified: typecheck clean, lint clean, prettier clean, check:outstanding-issues 125 rows / unique ids / next-id=128, and both affected tests still pass locally (2 passed, 12.6s). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 41 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 (4)
Comment |
BigSimmo
marked this pull request as ready for review
July 30, 2026 08:59
BigSimmo
added
the
skip-branch-sync
Claude
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e12bcb0cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…laim Codex review is correct and this retracts the previous commit's central claim. Reading `form.document-viewer-composer[data-scroll-hidden]` as a proxy for the header's `scrollHidden` was wrong: they are separate state machines. The header is driven by the shell's `chromeScrollHide` (global-search-shell.tsx:332), fed only by `useDocumentScrollHideReporter` (line 345) and passed in at line 876, while DocumentViewer runs its own two `useHideOnScroll` instances (use-document-viewer-chrome-scroll.ts:20-30). Composer-hidden therefore proves DocumentViewer's reporter fired and says nothing about the header's, so it never separated a pin from a reporter-never-fired — the exact distinction the helper claimed to make. `data-scroll-signal` on the collapse wrapper now publishes the header's raw `scrollHidden` before the pin is applied, and `expectChromeHidden` reports it alongside DocumentViewer's so a divergence between the two feeds is visible instead of collapsed into one verdict. Nothing styles the attribute; no CSS or code reads it (verified by grep), so behaviour is unchanged. Ledger #127 is corrected rather than patched over: the "traces prove sharedChromePinned is stuck" claim is explicitly withdrawn, what the traces do establish is separated from what they do not, and the new leading hypothesis is recorded as untested — the shell's feed is document-only, so where `#main-content` owns scrolling `window.scrollY` never moves and the shell reporter cannot see the gesture, which would explain the standalone-PWA variant directly. It also now says not to infer the header's scroll state from any page-owned composer. Verified: verify:cheap exit 0 — Test Files 434 passed (434), Tests 4562 passed | 4 skipped (4566); typecheck and lint clean; prettier clean; the full phone-scroll spec 56 passed (4.4m) against an isolated production build. That build used the container's Chromium 1194, not the bundled 1234 CI installs (#121), so it proves the attribute broke nothing and nothing more. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
Main's side carried #128-#130, archived #116 and reordered #117-#119, so it is taken whole; only the #127 row is grafted from this branch, since main still holds the withdrawn "sharedChromePinned is stuck" text. Marker stays at main's 131. The file now reports `union merge active`, so this class of collision should stop. Also cross-links #130 into #127. PR #1396 ("overlay the phone header so hiding it never moves content") merged at 06:49:55, about twenty minutes before the first failure, and rewrote the exact overlay path this defect sits in — phoneOverlayMotion, the overlay stack transition, and readChromeCollapseMetrics charging zero released top geometry. Its merge commit 90b3e34 is the single main run that passed, which fits the passed-once-then-failed-twice-escalating shape better than the focus-latch reading. #130 also records that #1396's own declared physical-device prerequisite was never run, and that headless Chromium cannot certify what it covers. Verified after the merge: check:outstanding-issues 128 rows / unique ids / next-id=131 / union merge active; check:branch-review-ledger 97 live + 1206 archived; prettier clean on all three changed files. Not re-run after the merge: verify:cheap and the phone-scroll spec, which both passed on this branch's own commit before it (434 test files / 4562 tests, and 56 passed). The merge adds only main's already-verified tree plus one docs cell, so the earlier evidence still covers this diff — but it is the earlier commit's evidence, not this one's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
…to claude/fix-shared-chrome-pin
`PR mergeability` flagged this branch, but `git merge-tree` returned a clean tree — behind-but-clean staleness, not a content conflict. The merge itself then reported success while producing every open-items row twice (`#59 appears 2 times (lines 101, 166)` and so on for the whole table): `.gitattributes` sets `merge=union` on this file, which is git's built-in concatenate-both-sides driver with no dedupe, and the table is not append-only. Rebuilt from `origin/main` (now through #134) with only the one row this branch actually changed re-applied. Recorded as #135, since the driver turns a resolvable conflict into a guaranteed guard failure and makes `merge-tree` look clean. Also corrects #127's own framing. `Production UI` PASSED on run 30530393684, so the failure is intermittent at 2 of 3 completed runs, not reproducible as the previous row claimed — that was premature on two datapoints. The `data-scroll-signal` diagnostic therefore has not yet had a failure to report; it is still the thing that will name the cause when one comes. Verified: check:outstanding-issues 133 rows / 67 open / unique ids / next-id=136; check:branch-review-ledger 112 live + 1206 archived; whole-tree prettier clean; no conflict markers under docs/, tests/ or src/. Not re-run for this merge: verify:cheap and the phone-scroll spec — the code changes are unchanged from 5495f28, where both passed (434 test files / 4562 tests, and 56 passed), and this commit touches only the ledger plus main's own already-verified tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
Third staleness resolution on this branch in about forty minutes. `PR mergeability` again reported `mergeable_state: dirty` and "a real merge conflict", but `git merge-tree --write-tree` returned a clean tree, so this is behind-but-clean staleness, not a content conflict. The single overlapping path between this branch and main is `docs/outstanding-issues.md` — nothing else on this branch is contested, which is why the code files are byte-identical to 5495f28. The file is rebuilt from `origin/main` with this branch's two rows re-applied (#127 corrected, #135 added after #134, marker 136) instead of keeping the union driver's output, which concatenates both sides of every overlapping hunk without dedupe and doubled the whole table last time — that behaviour is what #135 records. Main's #127 still carried the withdrawn "sharedChromePinned is stuck" text and #135 was unclaimed, so neither graft overwrote anyone else's edit. Verified: check:outstanding-issues 133 rows / unique ids / next-id=136; check:branch-review-ledger 113 live + 1206 archived; whole-tree prettier clean. Not re-run: verify:cheap and the phone-scroll spec — `git diff 5495f28 -- src/ tests/` is empty, so the code carries that commit's evidence (434 test files / 4562 tests, and 56 passed) unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
…it asks for A real conflict this time, not staleness: main's PR #1427 rewrote the same helpers this branch touches, and it very likely found the actual cause of #127. `addPhoneScrollRunway` slept 50ms and merely hoped the appended 1600px runway had reached layout; `dragScrollBy` clamped silently at the end of the range while reporting nothing. Under CI load the drag therefore delivered less than it asked for and the chrome was right to stay visible. #1427 polls for the runway, returns the distance actually travelled, and `dragScrollUntilHidden` refuses to expect a hide until remaining runway and delivered travel both clear 160px. Main's helpers are taken whole. This branch keeps only what #1427's own comment says is still missing: "Separating THOSE two still needs the pin state exposed in the DOM; today only the composite `data-scroll-hidden` (`scrollHidden && !sharedChromePinned`) is observable, so both look identical." `data-scroll-signal` publishes the raw signal, and `expectChromeHidden` is cut down to answer only that question, now layered after `dragScrollUntilHidden` rather than duplicating its travel proof. #127 is rewritten again and withdraws a second wrong diagnosis of my own: a short/clamped drag was ruled out early using a maxOffset of 2753 read at a different trace moment than the failing drag, when the pre-runway reading in that same trace was 1153 — and a runway not fully landed puts the offset in the near-bottom band where computeScrollHideUpdate legitimately refuses. That is exactly what #1427 fixes. The row now points at #1427 as the likely fix, keeps the observability gap as the only open part, and says to close it if no recurrence appears on a post-#1427 head. Main also claimed #135 for an unrelated issue, so the union-driver finding renumbers to #140, marker 141. Verified: typecheck 0 errors, lint 0 problems, whole-tree prettier clean, check:outstanding-issues 138 rows / unique ids / next-id=141, and the merged phone-scroll spec 56 passed (4.1m) against an isolated production build — under Chromium 1194, not CI's bundled 1234 (#121). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
# Conflicts: # tests/ui-phone-scroll.spec.ts
…to codex/review-pr1430 # Conflicts: # tests/ui-phone-scroll.spec.ts
4 tasks
BigSimmo
added a commit
that referenced
this pull request
Jul 30, 2026
* fix(ledger): remove merge=union from the issues ledger, per its own #133 Ledger #133 already recorded union as the wrong driver for this file — "two sides each bumping the marker produce two `next-id` lines, corrupting the file silently where a conflict would fail loudly" — but `.gitattributes` still set it and `check-outstanding-issues.mjs` *required* it, so the repo's own tested conclusion was contradicted by its own config. PR #1430 confirmed the cost at scale: four merges in one session, each reporting success while duplicating the entire open-items table (`#59 appears 2 times (lines 101, 166)` and so on for every row), each needing a manual rebuild from origin/main. Union also makes `git merge-tree` report a clean tree, so the pre-merge conflict check cannot warn. Unlike docs/branch-review-ledger.md — which keeps its custom `merge=ledger` driver, union plus exact-row dedupe — this file allocates IDs by read-modify-write. Concurrent appends therefore need manual renumbering whatever the driver does (hit twice on 2026-07-30: #125 and #135 collisions), so union bought nothing and only hid the overlap. Default 3-way merge conflicts honestly instead. The gate's attribute check is inverted rather than deleted, so a driver reappearing here is a red gate. AGENTS.md, docs/process-hardening.md, .claude/skills/issues/SKILL.md and docs/scripts-index.md are updated to match, and #133's driver half is marked resolved with its still-open half (fixed-width padding making every row edit one hunk) left intact. Verified: reintroducing `docs/outstanding-issues.md merge=union` fails the gate with "must have NO merge driver (found merge=union)", and removing it passes with "no merge driver" — the gate bites, not just passes. verify:cheap exit 0: Test Files 434 passed (434), Tests 4563 passed | 4 skipped (4567). Whole-tree prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * fix(gate): reject `-merge` too, not just a named driver Codex review is right: the new check accepted `unset` alongside `unspecified`, and those are not the same state. Per gitattributes, an Unspecified `merge` attribute is the documented default 3-way text merge — the contract this PR establishes — while Unset (`-merge`) takes the current branch's version and declares the merge conflicted, so every two-sided edit becomes a manual resolution. A global or future attributes file could therefore have violated the contract with the gate still printing "no merge driver". Reproduced before fixing: appending `docs/outstanding-issues.md -merge` made `git check-attr` report `merge: unset` and the guard passed. It now fails with a message naming the Unset/Unspecified distinction and telling the reader to drop the negated attribute rather than add one. The acceptance decision moves into an exported `mergeAttributeProblem` so the distinction is unit-tested rather than only reasoned about, with four cases in tests/repo-hygiene.test.ts: `unspecified` accepted; `unset`, `union`/`ledger`, and an empty reading all rejected. The empty case matters because an unparsed check-attr output would otherwise make the whole check vacuous. Verified: with `-merge` present the gate fails on the new message; with it removed it passes "no merge driver". repo-hygiene 47 passed (47). verify:cheap exit 0 — Test Files 435 passed (435), Tests 4508 passed | 4 skipped (4512). Whole-tree prettier clean. (Test total differs from this branch's earlier run because it now carries main's #1423/#1427/#1438; this commit adds four.) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ --------- Co-authored-by: Claude <noreply@anthropic.com>
3 tasks
BigSimmo
added a commit
that referenced
this pull request
Jul 30, 2026
* issues: close #140 as a duplicate of #133, resolved by #1444 #140 was opened mid-session for the union-driver damage before I noticed #133 had already recorded the same finding, earlier and with the same conclusion. Two open rows described one condition, and PR #1444 has since removed that condition: `merge=union` is gone from `.gitattributes`, `check:outstanding-issues` now requires an unspecified `merge` attribute, and regression tests cover `union`, `-merge` and an unparsed reading. Moved to the archive table rather than deleted, pointing readers at #133 — whose still-open half is the real conflict-frequency cause: fixed-width column padding makes any one-row edit re-pad every row, so git sees the whole table as one hunk. The surviving evidence (four merges on PR #1430 each reporting success while duplicating the entire open-items table) lives there too. Verified: check:outstanding-issues 138 rows, 66 open / 72 archived (was 67/71 — moved, not copied), unique ids, next-id=141, no merge driver; docs:check-links 1361 references resolve; prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * docs: record duplicate issue review --------- Co-authored-by: Claude <noreply@anthropic.com>
3 tasks
BigSimmo
added a commit
that referenced
this pull request
Jul 30, 2026
…ge gaps (#1496) * issues: close #130 by decision; capture the stale-install and reviewer gaps #130 is archived because the owner decided not to pursue it, NOT because the acceptance was run. The row says so in its first sentence, because a "CLOSED" line is exactly the thing a future reader could mistake for evidence: docs/phone-chrome-physical-acceptance.md on main is still the blank template, and nothing in CI covers what it covers — headless Chromium was explicitly stated as unable to certify Safari chrome-minimisation or cold-launch PWA paint. Residual risk is unchanged and unmeasured, and the row says to open a fresh one if phone-chrome misbehaves on a real device. Two risks that surfaced during today's work were never written down, and neither had an existing row (checked before adding, after creating a duplicate earlier today by not checking): #149 — the SessionStart hook reported "node_modules matches the lockfile" while brace-expansion was installed at 1.1.16 against a lockfile pinning 1.1.18, the CVE-2026-14257 patch. Every local gate run in that window was weaker than reported, including verify:cheap runs called green during #1430/#1444. It surfaced only because installed-lock-parity asserts the patched behaviour directly, and its failure was first misread as a pre-existing repo defect because it also failed on clean main in the same stale container. Notes that check:installed-lock-parity is only in verify:ui, so the broad local gate cannot catch its own stale foundation. #150 — CodeRabbit reviewed none of #1404, #1430, #1444, #1445 or #1479; every comment was a spending-cap rate-limit notice that renders as ordinary bot activity. Codex was the sole substantive reviewer and found three real defects that survived local gates and self-review. Fast checks run against this tree: check:runtime, check:installed-lock-parity (now passing, confirming the drift is resolved), check:outstanding-issues, check:branch-review-ledger and check:migration-role all PASS. check:supabase-project and check:production-readiness fail only on absent provider credentials, which AGENTS.md states is expected in demo mode. Verified: check:outstanding-issues 148 rows (57 open, 91 archived), unique ids, next-id=151, no merge driver, no ids deleted from base; docs:check-links clean; whole-tree prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * issues: correct #149 — parity check scope, not its placement Codex review is right on both counts and this rewrites the row rather than patching around it. The earlier text said check:installed-lock-parity lives only in verify:ui and proposed adding it to verify:cheap; verify:cheap:internal already runs it, so that follow-up was a no-op — and it ran green throughout the stale window. The real gap is scope. criticalInstalledPackages lists seven top-level names (next, react, react-dom, eslint, playwright, typescript, vitest), so a stale transitive dependency is invisible. Proved by faking node_modules/brace-expansion back to 1.1.16 against the 1.1.18 lockfile pin: check:installed-lock-parity exits 0. That also explains why both the hook and the gate reported parity while the tree was stale — neither was looking at the package that drifted. Next step is now to broaden the check (full tree, or a lockfile-hash install stamp) rather than move it, and the row warns against reading a green parity run as proof the install is current. Verified: check:outstanding-issues 148 rows, unique ids, next-id=151, no ids deleted from base; docs:check-links 1413 references; prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
data-scroll-hiddenon the collapse wrapper isscrollHidden && !sharedChromePinned, so a missing attribute has two very different causes the assertion cannot separate: the scroll state machine never fired, or it fired and a pin held the chrome open.expectChromeHiddenkeeps the identical pass condition and adds a failure message that says which.composerScrollHidden, which consultsscrollHiddenand not the pin — so composer-hidden plus header-visible proves the pin. Every term ofsharedChromePinnedalso has a DOM tell (anaria-expandedtrigger, a popover, or focus inside the portaled addon host), so when all of them read false the pin is a stale latch rather than a live surface.#127with the root cause the traces establish, replacing the "sticks visible under CI load" framing, which was wrong.What the traces establish
Production UIfailed on both completed full-suite runs —30521269873(one variant) and30524711173(both variants,2 failed, 339 passed) — always attests/ui-phone-scroll.spec.tsline 586, the reduced-motion hide following the section-sheet open/close, and never at line 486, the first hide.In the failing snapshot
form.document-viewer-composeranddocument-viewer-contentboth carrydata-scroll-hidden="true"whileuniversal-header-collapsedoes not. SoscrollHiddenis TRUE andsharedChromePinnedis stuck TRUE — the scroll state machine is working correctly, which rules out the load/flake reading.Narrowing which pin: the aria snapshot shows no open menu, sheet or popover, and
composerFocusPinsChromeadditionally requiresphoneBottomSearchDockActive, false on a route where DocumentViewer owns its composer. By elimination the stuck term isheaderChromeFocused, whoseheaderFocusPinsChromecarries no still-the-active-owner guard even thoughcomposerFocusPinsChromebeside it does — and whose comment atmaster-search-header.tsx:393describes precisely this latch. Document routes passstrategy: "collapse"(global-search-shell.tsx:864), so the addon-host focus handlers are live and the portaleddocument-section-triggeris the focus that latches.Not caused by any one PR:
mainchanged zero files undersrc/since the last passing run, and looks green only becauseProduction UIis skipped on its docs/CI-only pushes — it has not actually run this test since90b3e34.Why this is not the fix
Which value is stuck is proven; the latch mechanism is inferred. It does not reproduce locally, and
#121is why: 6/6 pass pinned to 2 CPUs and 56/56 for the whole file, but every local run used the container's Chromium build1194rather than the bundled1234CI installs, so no local green is evidence on this surface. Shipping a behaviour change to phone search chrome on inference is exactly whatdocs/search-chrome-behaviour.mdexists to prevent. This makes the next CI failure name its own cause instead of costing another trace download, and the fix follows once that datapoint lands.Verification
npm run typecheck— clean.npm run lint— clean, exit 0.npx prettier --check docs/outstanding-issues.md tests/ui-phone-scroll.spec.ts—All matched files use Prettier code style!npm run check:outstanding-issues—125 rows (61 open, 64 archived), unique ids, next-id=128 above the highest.2 passed (12.6s). Under Chromium1194, not CI's1234— this proves the helper did not break the passing path, and nothing more.npm run verify:phone-chromenot run: it selects owners/journeys and escalates toverify:ui, and every browser gate in this container runs the wrong Chromium build (#121), so its verdict here would be unearned. CI'sProduction UIis the real gate for this change.Risk and rollout
src/changes, so no production behaviour changes and the pass/fail condition is unchanged.Clinical Governance Preflight
Not applicable — no clinical-risk path is touched. The diff is one spec file and one docs row;
scripts/pr-policy.mjsclassifies nosrc/lib/**retrieval/answer/privacy surface, nosupabase/**and nosrc/app/api/**, and no RAG-ranking protected surface, so noRAG impact:declaration is required.Generated by Claude Code