Defer phone search suggestions until user types - #302
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
52d0f9c to
b4eb598
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Vector fallback drops global scope
- The vector fallback RPC now always forwards args.allowGlobalSearch (with per-filter documentIds) instead of passing undefined when a documentFilter is present.
- ✅ Fixed: Deferral applies beyond phone
- Suggestion deferral is now gated by an explicit requiresTypedQueryToOpen prop set from usesPhoneFooterDock, so desktop answer footer keeps focus-open recents behavior.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 1f95987. Configure here.
| owner_filter: ownerScopeForDocumentFilteredRetrieval( | ||
| args.ownerId, | ||
| documentFilter ? [documentFilter] : undefined, | ||
| documentFilter ? undefined : args.allowGlobalSearch, |
There was a problem hiding this comment.
Vector fallback drops global scope
Medium Severity
The match_document_chunks fallback path incorrectly passes undefined for allowGlobalSearch to ownerScopeForDocumentFilteredRetrieval when a documentFilter is present. This prevents anonymous searches with allowGlobalSearch: true from honoring the global search permission, causing them to fail or degrade via requireOwnerScope.
Reviewed by Cursor Bugbot for commit 1f95987. Configure here.
|
|
||
| function canOpenDropdownNow() { | ||
| return !requiresTypedQueryToOpen || trimmedQuery.length > 0; | ||
| } |
There was a problem hiding this comment.
Deferral applies beyond phone
Low Severity
Typed-query deferral is keyed on placement === "bottom-dock", but master-search-header assigns bottom-dock to every answer-mode footer composer, not only phone footer layouts. Desktop answer search loses focus-only recents and keyboard-open behavior that inline placement still provides, which goes beyond the PR’s phone-only intent.
Reviewed by Cursor Bugbot for commit 1f95987. Configure here.
- Pass allowGlobalSearch through document-scoped vector fallback RPCs - Scope requiresTypedQueryToOpen to phone footer dock, not all bottom-dock
docs/outstanding-issues.md has no merge driver by design, so this conflicted in seven hunks. Resolved by taking main's file as the base — it carries main's own archives (#207, #226, #230, #261, #284, #296) — then re-applying all 23 of this branch's changes through scripts/outstanding-issues.mjs: 15 archives and 8 detail rewrites. Verified afterwards that every one landed and that the guard reports no ids deleted from base. Two rows are deliberately NOT resurrected. This branch's #302 (style-contract line-break artifacts) and #303 (issues:next-id sync) no longer exist anywhere on main under any id — main has since reused both numbers for entirely different live rows. Both of this branch's rows were verified-delivered trivia, so nothing actionable is lost, but the disappearance is itself evidence for #156/#168: a main-side merge dropped rows, which is the hazard those rows describe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
Archived #303: probed the real CLI rather than reading it — ledger:append now accepts a --scope value beginning with a double dash. (The probe row it wrote was removed immediately and the guard re-run clean.) #302 re-measured and WORSE, which strengthens it: the legacyShadowAliases ceiling is still pinned at 220 while the gate measures 193 — 27 units of unguarded slack, up from 3 when the row was written. Paying #262's debt down bought headroom instead of tightening the gate, which is exactly the row's thesis. #215 re-scoped: one of its three items is done (image-lightbox has decoding=async) and the row's path was misleading — the file is under clinical-dashboard/, so a root-level search reads as 'already fixed'. Twenty stamped VERIFIED CORRECT with the measurement that settled each. Coverage: 58 of 119 open rows now carry sweep evidence, up from 34. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
…d rows The conflict was one hunk. Resolved by keeping this branch's #302 (re-measured 2026-08-12, newer than main's) and all four of main's genuinely new rows (#304, #305, #306, #307), dropping only main's older #302 and its #303, which this branch had already archived as delivered after probing the CLI. Correcting an earlier claim in this branch's history: #302 and #303 were NOT lost by main. Main renumbered them to #306 and #307 after the id collision, which is exactly what the guard advises. Both are re-filings of work already done, and both are archived here with evidence — style-contracts.ts:352-354 is clean, and the marker reads 308 against a highest declared id of 307. The derived loss detector reports one exception, #262, which is intentional: main's version of that row is newer than this branch's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
…bt rows (#1942) * refactor(tokens): re-land the --shadow-tight retirement onto --e1 PR #1803 retired the --shadow-tight role alias in favour of the --e1 elevation tier across 49 files and squash-merged as 9d8370a on 2026-08-10. The acf78bf merge on 2026-08-11 silently reverted it, along with six other PRs. This re-applies the retirement against current main: 130 call sites across 67 files, plus both declarations. The alias was a pure pass-through -- `--shadow-tight: var(--e1)` in the light and dark role blocks -- so the substitution is value-preserving. Confirmed for forced-colors too rather than assumed: the `@media (forced-colors: active)` block scopes `:root, .dark`, the same `html` element the alias is declared on, so `--shadow-tight` already resolved through the flattened `--e1: none` there. The .ckb-v2 redeclaration hazard does not bite for the same reason -- .ckb-v2 sits on <html> and .ckb-v2.ckb-v2 outspecifies :root, so both spellings substitute against the winning v2 tier. Two comments survived acf78bf while the code they describe did not: the globals.css note that "the resting-hairline role is gone", and the token test's "unlike the --shadow-tight assertion above". Both are accurate again. The token contract test now sweeps the tracked src tree for both spellings (declaration and var() consumer) instead of only asserting the declaration. A declaration-only check would have caught this particular revert, but only because the declarations happened to come back with the call sites; sweeping both makes the gate independent of which half of a bad merge lands. Mutation-verified in both directions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * chore(design-system): re-pin the contract ratchets to their measured values `scripts/design-system-contract-baseline.json` is a ceiling, so paying debt down leaves silent headroom behind. Ledger #302 records that pattern: legacyShadowAliases was pinned at 220 against a measured 193, 27 units of unguarded slack, up from 3 units on 2026-08-10. With the previous commit's --shadow-tight retirement applied the gap is wider still -- 220 pinned against 119 measured -- because the reland pays down the debt the acf78bf revert had re-hidden. Four other ratchets had accumulated slack from unrelated work in the same window. legacyShadowAliases 220 -> 119 edgeOwnershipConflicts 27 -> 25 rawPaddingLiterals 67 -> 63 rawGapLiterals 34 -> 32 layoutTransitionExceptions 12 -> 11 Regenerated with --print-debt-baseline rather than hand-edited, so the per-path debtByPath counts move with the totals -- those are what findDebtPathRegressions compares, and the retirement moved them wholesale. Every metric in the diff decreases; nothing is absorbed upward. This is not the baseline refresh #262 warns against. That stop rule forbids refreshing to hide the movement; this pins the movement in so it cannot silently drift back a second time. Mutation-verified: reintroducing one alias in button.tsx now fails at both the total (119 -> 120) and the per-path level. Under the old 220 ceiling the same addition passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * refactor(tokens): hold the search-band count bubble in a spacing token The active-filter badge sized itself with a raw `h-[1.0625rem] min-w-[1.0625rem]` pair. Ledger #275 tracks that value as leaked debt: it had reached five files, so the fix has always been to tokenise once rather than edit a call site. Re-measured on merged main, the badge role is down to a single call site. #170's convergence landed in the meantime -- document-search- results.tsx now renders the shared control and therapy-compass/ filter-sheet.tsx was deleted outright -- so the leak this row was written about has already been reabsorbed by the extraction. Holding the value in @theme is what stops it leaving again. Two arbitrary values in the same component are deliberately left raw: pr-[0.6875rem] and min-[414px]:max-[429px] -- the repo defines no --breakpoint-* tokens at all, and eight peer sites use the same raw min-[]/max-[] form (359px, 389px, 414px). Naming one window while the peers stay raw is the same drift #275 warns about on another axis, and Tailwind named breakpoints would add variants across the whole utility surface. That belongs in a repo-wide decision, filed separately. The three remaining 1.0625rem hits in mode-nav.tsx and nav-slot-ink.tsx are NOT this token. They size <Icon> glyphs -- a 17px icon against a 12/14/16/20/24 --spacing-icon-* scale -- so folding them under a badge token would merge two roles that only happen to share a number. check:icon-scale deliberately does not flag arbitrary h-[Nrem], so they are a real but separate finding, filed rather than guessed at. The token is also registered in CLINICAL_TWMERGE_THEME.spacing, which tests/tailwind-merge-config.test.ts asserts against the @theme block -- without it `cn()` cannot resolve a conflict on the new utility. Safe by that file's own `tap` reasoning: the single call site is a static string carrying no competing h-*/min-w-* class and never passes through `cn()`, so there is no same-variant pair for declaration to hand to the later class. The entry is protective for future use, not load-bearing today. Value-preserving, and proven rather than inferred: compiling globals.css through @tailwindcss/postcss emits .h-search-band-badge { height: var(--spacing-search-band-badge) } .min-w-search-band-badge { min-width: var(--spacing-search-band-badge) } No ratchet moved, so the ceilings pinned in the previous commit still sit at zero slack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * docs(design-system): close out DS Track A3 and refresh the stale gate rows Track A3 is `#262`. Its three parts are now all settled, each checked against code rather than against the row that describes it. Part 1 is the --shadow-tight retirement re-landed earlier in this PR. Part 3 shipped in PR #1780 per `#301`: rawPaddingLiterals, rawRadiusLiterals and rawLineHeightLiterals are live baseline keys enforced over both the class and CSS-declaration spellings, plus rawGapLiterals beyond the original ask. Part 2 needs no work, and that had already been adjudicated -- GATES.md section 3 records it, which is why nothing here builds it. The decidable half of step selection shipped on 9 Aug inside check:design-system- contract: a declared @theme step no production surface selects fails the build. The remaining half -- which existing step a component picks -- is documented there as something "nothing mechanical can" gate, being a judgement about the rendered design rather than a property of the source, with a standing instruction not to duplicate the arbitrary-value check check:type-scale already ships. Reading `#262` alone would have sent a session to build it; that is the `#301` failure mode, so the closure record says so explicitly. Section 3's live status rows carried numbers this PR moved. `#301`'s lesson is that a row understating shipped work is a duplicate-work generator, so they are corrected in the same change: legacyShadowAliases 224 -> 119, and the alias is now retired outright rather than "224 left to retire" edgeOwnershipConflicts 27 -> 25 rawPaddingLiterals 67 -> 63 rawGapLiterals 34 -> 32 layoutTransitionExceptions 12 -> 11 Section 5 is left alone deliberately: it is a dated record measured against 8db1e53, not a live status surface, and rewriting its figures would destroy the provenance it exists to hold. Ledger records are queued as immutable inbox requests: `#262`, `#302` and `#275` closed; two carve-outs split out of `#275` filed as their own rows (the repo-wide breakpoint-token decision, and three 17px mode-nav icon glyphs that sit off the --spacing-icon-* scale with no gate covering them). The queued re-land request 210e3db5 is cancelled rather than reconciled -- its headline "67 files on main still use the retired alias" is false as of this branch, so it would open a row wrong on arrival. The request file and the cancellation both survive as provenance for the acf78bf merge loss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * chore(ledger): record the design-token relands review Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * fix(issues): retarget the reland record after main reconciled it mid-flight CI failed `docs:check-links` on this branch with Error: cancel request 2e791c01... targets missing pending request 210e3db5... `check-docs-links.mjs` replays the inbox batch to resolve link targets, so an unresolvable request fails it. The cause was a race, not a bad record: PR #1936 reconciled 75 queued requests -- 210e3db5 among them -- while this branch was already in flight. Reconciling moves the request file into `docs/outstanding-issues-inbox/applied/` and allocates it a canonical row, so by the time this branch merged main there was no pending request left for the cancellation to name. Cancelling was the right call against a pending request and is the wrong one against a reconciled row. The cancel is dropped and replaced with a `done` against `#319`, the row 210e3db5 became. That is also the better record: the work is finished rather than withdrawn, so the ledger should carry its outcome and its guard, which a cancellation would have thrown away. Also merges origin/main (this branch was 3 behind) and files two findings the PR preflight surfaced, both deliberately not fixed here: - `check:medication-lexicon-report` has been failing on main for every local `verify:pr-local`, and no CI job runs it -- a grep over .github/workflows finds nothing. It is the last step of the local chain, so it fails preflights while CI stays green. The stale file is a clinical-facing generated document; regenerating it inside a CSS-token PR would bundle a clinical-risk artefact with unrelated chores. - Claude Code web containers can ship Node 22 with no node_modules, which fails `npm ci` on engine-strict before any repo script can run. Re-verified after the merge: the tracked tree still holds zero `--shadow-tight` references, and every pinned ratchet still measures exactly its baseline, so the merge moved no metric and the pins stay honest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * docs(design): retire shadow-tight guidance * docs(design): retire shadow-tight guidance * docs(design): retire shadow-tight guidance * docs(design): retire shadow-tight guidance --------- Co-authored-by: Claude <noreply@anthropic.com>


Summary
On phone devices, the bottom-dock search composer no longer expands with suggestions when the input is merely focused or tapped. The smart dropdown now opens only after the user starts typing.
Merged latest
cursor/remove-answer-bar-chips-8b1aand resolved conflicts.Changes
universal-search-command-surface.tsx: Forbottom-dockplacement, defer opening the command dropdown until the query has text. Preserves upstreamAnswerSuggestionChipsrefactor for the examples hint row.tests/ui-tools.spec.ts: Assert listbox stays closed on focus/ArrowDown with empty query, then opens after typing.Conflict resolution
Both conflicts were simple (no conflicting product intent):
useReffor typed-query gating; dropped unuseduseSyncExternalStorefrom upstream chip refactor.