skills/backnotprop__plannotator
1
0
Fork 0
mirror of https://github.com/backnotprop/plannotator.git synced 2026-09-14 14:17:26 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat(review): hover delay default is 300ms matching VS Code; tiers respaced 150/300/700

Browse Source
This commit is contained in:
Michael Ramos
2026-09-02 22:59:24 -07:00
parent 50f8e22781
commit b55383502d
7 changed files with 48 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
AGENTS.md
+1 -1
View File
@@ -553,7 +553,7 @@ During normal plan review, an Archive sidebar tab provides the same browsing via
| `/api/guide/:jobId/output` | GET | Fetch a failed guide job's captured raw output for manual repair (404 if none captured) |
| `/api/guide/:jobId/submit` | POST | Manually submit corrected guide JSON for a failed job (body: `{ payload }`) |
| `/api/code-nav/resolve` | POST | Search for symbol definitions and references via ripgrep (body: `{ symbol, filePath, line, charStart, side, language? }`) |
| `/api/code-nav/hover` | POST | Token hover card resolution: the same body and the same guards as `/resolve`, over the same ripgrep search, returning one enriched definition (kind, approximate signature, heuristic doc comment, short preview), an optional runner-up candidate, and a five-reference sample. `backend: 'unavailable'` (rg missing) is a normal 200 and the client renders nothing. `source` is always `'search'` in Tier 0; the nullable `symbolKind` / `signature` / `doc` fields are what a later syntax- or index-backed tier would fill. Client-side the card is governed by two cookie-only settings (`packages/ui/config/settings.ts`, types in `@plannotator/core/token-hover`): `tokenHoverTrigger` (`hover` default / `modifier`, meaning the platform's primary modifier held, Cmd on macOS and Ctrl elsewhere via `isModKeyHeld` / `modEventKey` in `packages/ui/utils/platform.ts` / `off`, which withholds the handler props so the diff views wire no listeners at all) and `tokenHoverDelay` (200 / 350 default / 700 ms, the dwell before any request exists). `tokenHoverTrigger` REPLACED the original `tokenHoverCards` boolean and re-reads its cookie as a migration on every load until the user touches the setting (`false` becomes `off`), never writing it back: a migrating read returns a value, so the registry's default-seeding write never fires, and resolution is pure and identical every time. Cmd rather than Alt because Alt is widely bound to push-to-talk dictation (an Alt gate would open cards while the user speaks) and because Cmd+hover is already VS Code's "tell me about this symbol" gesture; the navigable-target underline and the card under one held key is that composite gesture, not a collision, since `handleCodeNavRequest` dismisses the hover surface on every References invocation. Only the modifier ALONE arms: any other key while it is held (Cmd+C) disarms and closes, so a copy with the pointer parked on a token cannot pop a card. Click behavior is untouched by the trigger setting, and the #1461 Alt+click References alias is unrelated. The stored value stays `modifier`: the setting names the shape of the gate, not which key fills it. |
| `/api/code-nav/hover` | POST | Token hover card resolution: the same body and the same guards as `/resolve`, over the same ripgrep search, returning one enriched definition (kind, approximate signature, heuristic doc comment, short preview), an optional runner-up candidate, and a five-reference sample. `backend: 'unavailable'` (rg missing) is a normal 200 and the client renders nothing. `source` is always `'search'` in Tier 0; the nullable `symbolKind` / `signature` / `doc` fields are what a later syntax- or index-backed tier would fill. Client-side the card is governed by two cookie-only settings (`packages/ui/config/settings.ts`, types in `@plannotator/core/token-hover`): `tokenHoverTrigger` (`hover` default / `modifier`, meaning the platform's primary modifier held, Cmd on macOS and Ctrl elsewhere via `isModKeyHeld` / `modEventKey` in `packages/ui/utils/platform.ts` / `off`, which withholds the handler props so the diff views wire no listeners at all) and `tokenHoverDelay` (150 / 300 default / 700 ms, the dwell before any request exists; 300 matches the VS Code hover delay). `tokenHoverTrigger` REPLACED the original `tokenHoverCards` boolean and re-reads its cookie as a migration on every load until the user touches the setting (`false` becomes `off`), never writing it back: a migrating read returns a value, so the registry's default-seeding write never fires, and resolution is pure and identical every time. Cmd rather than Alt because Alt is widely bound to push-to-talk dictation (an Alt gate would open cards while the user speaks) and because Cmd+hover is already VS Code's "tell me about this symbol" gesture; the navigable-target underline and the card under one held key is that composite gesture, not a collision, since `handleCodeNavRequest` dismisses the hover surface on every References invocation. Only the modifier ALONE arms: any other key while it is held (Cmd+C) disarms and closes, so a copy with the pointer parked on a token cannot pop a card. Click behavior is untouched by the trigger setting, and the #1461 Alt+click References alias is unrelated. The stored value stays `modifier`: the setting names the shape of the gate, not which key fills it. |
| `/api/code-nav/file` | GET | Read file from working tree for code-nav preview (`?path=`) |
### Annotate Server (`packages/server/annotate.ts`)
DESIGN_hover-settings.md
+2 -2
View File
@@ -265,7 +265,7 @@ flag clears and the card closes.
the mode is `modifier`. In `hover` mode the pipeline is byte-for-byte what
#1461 shipped, plus a delay constant read from settings.
**Delay.** `DWELL_MS` becomes the hook's `delayMs` option, defaulting to 350 so
**Delay.** `DWELL_MS` becomes the hook's `delayMs` option, defaulting to 300 (matching VS Code, maintainer ruling; tiers respaced 150/300/700 to keep Fast perceptibly distinct) so
every existing call site and test is unchanged. The leave grace, the cache
size, the scroll cancel and the render threshold are untouched.
@@ -455,7 +455,7 @@ Per the repo's testing rules, each of these names a failure it catches.
cannot be dismissed.
3b. **A chord opens nothing and closes an open card**: Cmd+C with the pointer
parked on a token must not pop a card mid-copy.
4. **Delay feeds the dwell** (same suite): a 700 delay spawns nothing at 350.
4. **Delay feeds the dwell** (same suite): a 700 delay spawns nothing at 300.
Catches the constant being left hardcoded.
5. **Legacy cookie migration** (new `tokenHoverSetting.test.ts`, pure lane):
`plannotator-token-hover-cards=false` resolves to `off`; `true` and absent
packages/core/token-hover.ts
+5 -5
View File
@@ -49,20 +49,20 @@ export function isTokenHoverTrigger(value: unknown): value is TokenHoverTrigger
* than a slider: the perceptible granularity here is around 150ms, so offering
* finer precision would imply a difference nobody can feel.
*/
export type TokenHoverDelay = 200 | 350 | 700;
export type TokenHoverDelay = 150 | 300 | 700;
/** The dwell #1461 shipped, and still the default. */
export const DEFAULT_TOKEN_HOVER_DELAY_MS = 350 satisfies TokenHoverDelay;
export const DEFAULT_TOKEN_HOVER_DELAY_MS = 300 satisfies TokenHoverDelay;
/** Every delay, fastest first, in the order the Settings control presents them. */
export const TOKEN_HOVER_DELAYS = [
200,
350,
150,
300,
700,
] as const satisfies readonly TokenHoverDelay[];
export function isTokenHoverDelay(value: unknown): value is TokenHoverDelay {
return value === 200 || value === 350 || value === 700;
return value === 150 || value === 300 || value === 700;
}
/**
packages/review-editor/hooks/useTokenHover.test.tsx
+34 -34
View File
@@ -169,7 +169,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(349); });
await act(async () => { jest.advanceTimersByTime(299); });
expect(pending).toHaveLength(0);
await act(async () => { jest.advanceTimersByTime(2); });
@@ -190,7 +190,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
test('a new symbol supersedes and aborts the in-flight request', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
await act(async () => {
@@ -198,7 +198,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
});
expect(pending[0].signal?.aborted).toBe(true);
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(2);
expect(pending[1].request.symbol).toBe('withRetry');
});
@@ -206,7 +206,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
test('the card survives the leave grace and entering it cancels the close', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(latest!.hover).not.toBeNull();
@@ -228,13 +228,13 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
test('a re-hover inside the cache spawns no second request', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
await act(async () => latest!.onTokenHoverLeave());
await act(async () => { jest.advanceTimersByTime(250); });
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
expect(latest!.hover).not.toBeNull();
@@ -243,7 +243,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
test('an unavailable backend renders nothing', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse({
backend: 'unavailable',
definition: null,
@@ -257,7 +257,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
test('an answer with no definition and one reference renders nothing', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse({
definition: null,
references: [{ filePath: 'src/other.js', line: 4, column: 2, snippet: 'charge(1)' }],
@@ -275,14 +275,14 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
await mount();
const tokenX = token();
await act(async () => latest!.onTokenHoverEnter(REQUEST, tokenX));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(latest!.hover!.data.symbol).toBe('charge');
const neighbour = { ...REQUEST, symbol: 'withRetry' };
await act(async () => latest!.onTokenHoverLeave());
await act(async () => latest!.onTokenHoverEnter(neighbour, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(2);
// Back onto the still-open card's token, then the neighbour answers.
@@ -302,13 +302,13 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
await mount();
const tokenX = token();
await act(async () => latest!.onTokenHoverEnter(REQUEST, tokenX));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
await act(async () => latest!.onTokenHoverLeave());
await act(async () => { jest.advanceTimersByTime(100); });
await act(async () => latest!.onTokenHoverEnter(REQUEST, tokenX));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
expect(pending[0].signal?.aborted).toBe(false);
@@ -321,7 +321,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
test('a below-threshold answer for another token closes the stale card', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(latest!.hover).not.toBeNull();
@@ -329,7 +329,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
await act(async () => {
latest!.onTokenHoverEnter({ ...REQUEST, symbol: 'gateway' }, token());
});
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(1, hoverResponse({
symbol: 'gateway',
definition: null,
@@ -348,7 +348,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
await mount();
const recycled = token();
await act(async () => latest!.onTokenHoverEnter(REQUEST, recycled));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
recycled.remove();
await settle(0, hoverResponse());
@@ -358,7 +358,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
test('scrolling inside the card leaves it open; scrolling the pane closes it', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
// The signature block is a horizontal scroller: reading a long signature
@@ -386,7 +386,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
// cached answer must never be served across a refresh.
await mount('snapshot-1');
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(pending).toHaveLength(1);
@@ -396,14 +396,14 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
expect(latest!.hover).toBeNull();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(2);
});
test('unmounting abandons the pending request', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
const current = root!;
@@ -416,7 +416,7 @@ describe.skipIf(!hasDom)('useTokenHover', () => {
test('scrolling closes the card and abandons the pending request', async () => {
await mount();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
await act(async () => {
@@ -448,7 +448,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
await modDown();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
await settle(0, hoverResponse());
expect(latest!.hover?.request.symbol).toBe('charge');
@@ -462,7 +462,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
expect(pending).toHaveLength(0);
await modDown();
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
});
@@ -473,7 +473,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
await act(async () => latest!.onTokenHoverLeave());
await modDown();
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(0);
});
@@ -482,7 +482,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
await mount('snapshot-1', { mode: 'modifier' });
await modDown();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(latest!.hover).not.toBeNull();
@@ -497,7 +497,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
await mount('snapshot-1', { mode: 'modifier' });
await modDown();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
await act(async () => latest!.onCardEnter());
@@ -542,7 +542,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
await mount('snapshot-1', { mode: 'modifier' });
await modDown();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(latest!.hover).not.toBeNull();
@@ -561,7 +561,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
await mount('snapshot-1', { mode: 'modifier' });
await modDown();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
await act(async () => latest!.onCardEnter());
@@ -572,7 +572,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
// A fresh card on another token, then a release.
const second = { ...REQUEST, symbol: 'refund' };
await act(async () => latest!.onTokenHoverEnter(second, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(1, hoverResponse({ symbol: 'refund' }));
expect(latest!.hover).not.toBeNull();
@@ -588,7 +588,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
await mount('snapshot-1', { mode: 'modifier' });
await modDown();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(latest!.hover).not.toBeNull();
@@ -604,7 +604,7 @@ describe.skipIf(!hasDom)('useTokenHover trigger mode', () => {
test('hover mode ignores the key entirely', async () => {
await mount('snapshot-1', { mode: 'hover' });
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(latest!.hover).not.toBeNull();
@@ -632,7 +632,7 @@ describe.skipIf(!hasDom)('useTokenHover References handoff', () => {
await mount('snapshot-1', { mode: 'modifier' });
await modDown();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
await settle(0, hoverResponse());
expect(latest!.hover).not.toBeNull();
@@ -661,7 +661,7 @@ describe.skipIf(!hasDom)('useTokenHover References handoff', () => {
await mount('snapshot-1', { mode: 'modifier' });
await modDown();
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
await act(async () => latest!.close());
@@ -677,10 +677,10 @@ describe.skipIf(!hasDom)('useTokenHover delay', () => {
await mount('snapshot-1', { delayMs: 700 });
await act(async () => latest!.onTokenHoverEnter(REQUEST, token()));
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(0);
await act(async () => { jest.advanceTimersByTime(350); });
await act(async () => { jest.advanceTimersByTime(300); });
expect(pending).toHaveLength(1);
});
});
packages/review-editor/hooks/useTokenHover.ts
+1 -1
View File
@@ -125,7 +125,7 @@ function meetsRenderThreshold(data: CodeNavHoverResponse): boolean {
* @param snapshotId the active diff snapshot; a change flushes the cache so a
* refreshed diff can never serve positions from the diff before it.
* @param options the user's trigger mode and dwell. Omitted means the shipped
* hover-at-350ms behavior.
* hover-at-default-delay behavior.
*/
export function useTokenHover(
snapshotId?: string,
packages/ui/components/Settings.tsx
+3 -3
View File
@@ -145,10 +145,10 @@ export const TOKEN_HOVER_TRIGGER_OPTIONS = [
{ value: 'off' as const, label: 'Off' },
];
/** SegmentedControl keys on strings, so the ms values ride as their digits. */
export type TokenHoverDelayOption = '200' | '350' | '700';
export type TokenHoverDelayOption = '150' | '300' | '700';
export const TOKEN_HOVER_DELAY_OPTIONS = [
{ value: '200' as const, label: 'Fast' },
{ value: '350' as const, label: 'Default' },
{ value: '150' as const, label: 'Fast' },
{ value: '300' as const, label: 'Default' },
{ value: '700' as const, label: 'Relaxed' },
];
export const OVERFLOW_OPTIONS = [
packages/ui/config/tokenHoverSetting.test.ts
+2 -2
View File
@@ -79,14 +79,14 @@ describe('tokenHoverTrigger', () => {
describe('tokenHoverDelay', () => {
test('defaults to the dwell the feature shipped with', () => {
installBackend();
expect(SETTINGS.tokenHoverDelay.defaultValue).toBe(350);
expect(SETTINGS.tokenHoverDelay.defaultValue).toBe(300);
expect(SETTINGS.tokenHoverDelay.fromCookie()).toBeUndefined();
});
test('round-trips the three steps as numbers and rejects anything else', () => {
const values = installBackend();
for (const delay of [200, 350, 700] as const) {
for (const delay of [150, 300, 700] as const) {
SETTINGS.tokenHoverDelay.toCookie(delay);
expect(values.get(DELAY_KEY)).toBe(String(delay));
expect(SETTINGS.tokenHoverDelay.fromCookie()).toBe(delay);