refactor(fs): minimize and cap search card meta; keep TUI byte-identical
Address the review of the search render card: - The search result view carries no `content`: it was a no-op for every consumer and serialized the whole search text twice. A UI without a search card falls back to the raw tool/result content; the TUI stays byte-identical to the pre-search-card generic fallback. - Bound the serialized presentationMeta with a configurable searchMetaMaxBytes (default 64 KiB): the inline item cap does not bound bytes, and spill-policy only shrinks content, never meta. capMetaBytes drops trailing groups/paths. - Share one retention pass (retainGrepMatches/retainGlobPaths in search-core) between the model-facing render and the meta projection; remove the second cap/preview implementation and the presentation<->grep module cycle by moving GrepMatch/previewLine to search-core. - Rename the result-view discriminant kind -> shape so it no longer collides with GenericCallView.kind (ToolCallKind, whose values include 'search'). - Narrow the entry export surface to consumed symbols. - Sync the three bilingual ToolResultView doc pairs and the Agent Note pair; document the deliberate empty-card acceptance vs diffsFromMeta. - Regenerate config/tool/cordis catalogs for the new config field.
This commit is contained in:
Chinesezjc
@@ -2,9 +2,10 @@
|
||||
* Unit tests for the search-card presentation layer (`src/presentation.ts`): the
|
||||
* canonical value → `presentationMeta` projections (`grepSearchMeta`,
|
||||
* `globSearchMeta`, `groupMatchesByFile`) and the defensive `meta` → view
|
||||
* narrowing (`searchViewFromMeta`). These pin the by-file grouping, the inline
|
||||
* cap and `truncated`/`total` honesty, and the malformed-metadata fallback a
|
||||
* replayed or hand-edited log can deliver.
|
||||
* narrowing (`searchViewFromMeta`). These pin the by-file grouping, the
|
||||
* `truncated`/`total` honesty over already-retained input, the serialized-meta
|
||||
* byte cap, and the malformed-metadata fallback a replayed or hand-edited log can
|
||||
* deliver.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from 'vitest'
|
||||
@@ -15,10 +16,14 @@ import {
|
||||
groupMatchesByFile,
|
||||
searchViewFromMeta,
|
||||
} from '../src/presentation.ts'
|
||||
import type { GrepMatch } from '../src/grep.ts'
|
||||
import type { GrepMatch } from '../src/search-core.ts'
|
||||
import { retainGlobPaths, retainGrepMatches } from '../src/search-core.ts'
|
||||
|
||||
const match = (path: string, lineNumber: number, line: string): GrepMatch => ({ path, lineNumber, line })
|
||||
|
||||
/** A byte cap large enough that no test payload here is meta-capped. */
|
||||
const WIDE = 1_000_000
|
||||
|
||||
describe('groupMatchesByFile', () => {
|
||||
it('groups matches by first-seen file order, keeping line/lineNumber only', () => {
|
||||
expect(groupMatchesByFile([
|
||||
@@ -38,38 +43,73 @@ describe('groupMatchesByFile', () => {
|
||||
|
||||
describe('grepSearchMeta', () => {
|
||||
it('projects grouped matches with total and a false truncation flag within the cap', () => {
|
||||
const meta = grepSearchMeta([match('a.ts', 1, 'one'), match('a.ts', 2, 'two')], 10, 2000)
|
||||
const meta = grepSearchMeta(retainGrepMatches([match('a.ts', 1, 'one'), match('a.ts', 2, 'two')], 10, 2000), WIDE)
|
||||
expect(meta).toEqual({
|
||||
kind: 'matches',
|
||||
shape: 'matches',
|
||||
files: [{ path: 'a.ts', matches: [{ lineNumber: 1, line: 'one' }, { lineNumber: 2, line: 'two' }] }],
|
||||
truncated: false,
|
||||
total: 2,
|
||||
})
|
||||
})
|
||||
|
||||
it('caps the retained matches and reports the pre-cap total when truncated', () => {
|
||||
const meta = grepSearchMeta([match('a.ts', 1, 'one'), match('a.ts', 2, 'two'), match('b.ts', 3, 'three')], 2, 2000)
|
||||
it('reports the pre-cap total and truncation from the shared retention pass', () => {
|
||||
const meta = grepSearchMeta(retainGrepMatches([match('a.ts', 1, 'one'), match('a.ts', 2, 'two'), match('b.ts', 3, 'three')], 2, 2000), WIDE)
|
||||
expect(meta).toEqual({
|
||||
kind: 'matches',
|
||||
shape: 'matches',
|
||||
files: [{ path: 'a.ts', matches: [{ lineNumber: 1, line: 'one' }, { lineNumber: 2, line: 'two' }] }],
|
||||
truncated: true,
|
||||
total: 3,
|
||||
})
|
||||
})
|
||||
|
||||
it('applies the per-line preview budget (UTF-8 boundary) to the projected line', () => {
|
||||
const meta = grepSearchMeta([match('a.txt', 1, 'aéaéaéaé')], 10, 7)
|
||||
expect(meta).toMatchObject({ kind: 'matches', files: [{ path: 'a.txt', matches: [{ lineNumber: 1, line: 'aéaéa (line truncated)' }] }] })
|
||||
it('carries the per-line preview budget (UTF-8 boundary) the retention pass applied', () => {
|
||||
const meta = grepSearchMeta(retainGrepMatches([match('a.txt', 1, 'aéaéaéaé')], 10, 7), WIDE)
|
||||
expect(meta).toMatchObject({ shape: 'matches', files: [{ path: 'a.txt', matches: [{ lineNumber: 1, line: 'aéaéa (line truncated)' }] }] })
|
||||
})
|
||||
|
||||
it('drops trailing file groups until the serialized meta fits the byte cap, marking it truncated', () => {
|
||||
const retained = retainGrepMatches(
|
||||
[match('a.ts', 1, 'x'.repeat(60)), match('b.ts', 2, 'y'.repeat(60)), match('c.ts', 3, 'z'.repeat(60))],
|
||||
10,
|
||||
2000,
|
||||
)
|
||||
// One 60-byte group serializes to ~110 bytes; a 260-byte cap holds two, not three.
|
||||
const meta = grepSearchMeta(retained, 260)
|
||||
expect(meta.shape).toBe('matches')
|
||||
if (meta.shape !== 'matches') throw new Error('unreachable')
|
||||
expect(meta.truncated).toBe(true)
|
||||
expect(meta.total).toBe(3)
|
||||
expect(meta.files.length).toBeLessThan(3)
|
||||
expect(Buffer.byteLength(JSON.stringify(meta), 'utf8')).toBeLessThanOrEqual(260)
|
||||
})
|
||||
|
||||
it('keeps a single oversized group rather than emit an empty card', () => {
|
||||
const meta = grepSearchMeta(retainGrepMatches([match('a.ts', 1, 'x'.repeat(500))], 10, 2000), 50)
|
||||
expect(meta.shape).toBe('matches')
|
||||
if (meta.shape !== 'matches') throw new Error('unreachable')
|
||||
expect(meta.files).toHaveLength(1)
|
||||
expect(meta.truncated).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('globSearchMeta', () => {
|
||||
it('projects the path list with total and a false truncation flag within the cap', () => {
|
||||
expect(globSearchMeta(['a.ts', 'b.ts'], 10)).toEqual({ kind: 'paths', paths: ['a.ts', 'b.ts'], truncated: false, total: 2 })
|
||||
expect(globSearchMeta(retainGlobPaths(['a.ts', 'b.ts'], 10), WIDE)).toEqual({ shape: 'paths', paths: ['a.ts', 'b.ts'], truncated: false, total: 2 })
|
||||
})
|
||||
|
||||
it('caps the retained paths and reports the pre-cap total when truncated', () => {
|
||||
expect(globSearchMeta(['a.ts', 'b.ts', 'c.ts'], 2)).toEqual({ kind: 'paths', paths: ['a.ts', 'b.ts'], truncated: true, total: 3 })
|
||||
it('reports the pre-cap total and truncation from the shared retention pass', () => {
|
||||
expect(globSearchMeta(retainGlobPaths(['a.ts', 'b.ts', 'c.ts'], 2), WIDE)).toEqual({ shape: 'paths', paths: ['a.ts', 'b.ts'], truncated: true, total: 3 })
|
||||
})
|
||||
|
||||
it('drops trailing paths until the serialized meta fits the byte cap, marking it truncated', () => {
|
||||
const retained = retainGlobPaths([`${'a'.repeat(100)}.ts`, `${'b'.repeat(100)}.ts`, `${'c'.repeat(100)}.ts`], 10)
|
||||
const meta = globSearchMeta(retained, 180)
|
||||
expect(meta.shape).toBe('paths')
|
||||
if (meta.shape !== 'paths') throw new Error('unreachable')
|
||||
expect(meta.truncated).toBe(true)
|
||||
expect(meta.total).toBe(3)
|
||||
expect(meta.paths.length).toBeLessThan(3)
|
||||
expect(Buffer.byteLength(JSON.stringify(meta), 'utf8')).toBeLessThanOrEqual(180)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -80,15 +120,22 @@ describe('searchViewFromMeta (defensive narrowing)', () => {
|
||||
const m = (value: unknown): JsonValue | undefined => value as JsonValue | undefined
|
||||
|
||||
it('narrows a well-formed matches payload into a matches view', () => {
|
||||
const meta = { kind: 'matches', files: [{ path: 'a.ts', matches: [{ lineNumber: 1, line: 'x' }] }], truncated: true, total: 5 }
|
||||
const meta = { shape: 'matches', files: [{ path: 'a.ts', matches: [{ lineNumber: 1, line: 'x' }] }], truncated: true, total: 5 }
|
||||
expect(searchViewFromMeta(m(meta))).toEqual({ card: 'search', ...meta })
|
||||
})
|
||||
|
||||
it('narrows a well-formed paths payload into a paths view', () => {
|
||||
const meta = { kind: 'paths', paths: ['a.ts', 'b.ts'], truncated: false, total: 2 }
|
||||
const meta = { shape: 'paths', paths: ['a.ts', 'b.ts'], truncated: false, total: 2 }
|
||||
expect(searchViewFromMeta(m(meta))).toEqual({ card: 'search', ...meta })
|
||||
})
|
||||
|
||||
it('narrows a zero-result payload into a valid empty card (not a rejected projection)', () => {
|
||||
expect(searchViewFromMeta(m({ shape: 'matches', files: [], truncated: false, total: 0 })))
|
||||
.toEqual({ card: 'search', shape: 'matches', files: [], truncated: false, total: 0 })
|
||||
expect(searchViewFromMeta(m({ shape: 'paths', paths: [], truncated: false, total: 0 })))
|
||||
.toEqual({ card: 'search', shape: 'paths', paths: [], truncated: false, total: 0 })
|
||||
})
|
||||
|
||||
it('rejects undefined / non-object / array meta', () => {
|
||||
expect(searchViewFromMeta(undefined)).toBeUndefined()
|
||||
expect(searchViewFromMeta(null)).toBeUndefined()
|
||||
@@ -97,19 +144,19 @@ describe('searchViewFromMeta (defensive narrowing)', () => {
|
||||
})
|
||||
|
||||
it('rejects a payload with a missing / mistyped truncated or total field', () => {
|
||||
expect(searchViewFromMeta(m({ kind: 'paths', paths: [], total: 0 }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ kind: 'paths', paths: [], truncated: 'no', total: 0 }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ kind: 'paths', paths: [], truncated: false }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ kind: 'paths', paths: [], truncated: false, total: '0' }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ shape: 'paths', paths: [], total: 0 }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ shape: 'paths', paths: [], truncated: 'no', total: 0 }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ shape: 'paths', paths: [], truncated: false }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ shape: 'paths', paths: [], truncated: false, total: '0' }))).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects an unknown or missing kind discriminant', () => {
|
||||
expect(searchViewFromMeta(m({ kind: 'other', truncated: false, total: 0 }))).toBeUndefined()
|
||||
it('rejects an unknown or missing shape discriminant', () => {
|
||||
expect(searchViewFromMeta(m({ shape: 'other', truncated: false, total: 0 }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ truncated: false, total: 0 }))).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects a matches payload with a malformed files array', () => {
|
||||
const base = { kind: 'matches', truncated: false, total: 1 }
|
||||
const base = { shape: 'matches', truncated: false, total: 1 }
|
||||
expect(searchViewFromMeta(m({ ...base, files: 'x' }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ ...base, files: [null] }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ ...base, files: ['x'] }))).toBeUndefined()
|
||||
@@ -122,7 +169,7 @@ describe('searchViewFromMeta (defensive narrowing)', () => {
|
||||
})
|
||||
|
||||
it('rejects a paths payload with a non-array or non-string-element paths field', () => {
|
||||
const base = { kind: 'paths', truncated: false, total: 1 }
|
||||
const base = { shape: 'paths', truncated: false, total: 1 }
|
||||
expect(searchViewFromMeta(m({ ...base, paths: 'x' }))).toBeUndefined()
|
||||
expect(searchViewFromMeta(m({ ...base, paths: [1] }))).toBeUndefined()
|
||||
})
|
||||
|
||||
@@ -817,7 +817,7 @@ describe('presentation', () => {
|
||||
if (result.isError) throw new Error('expected grep success')
|
||||
// The presentationMeta projection rides the result meta (a surface call).
|
||||
expect(result.meta).toEqual({
|
||||
kind: 'matches',
|
||||
shape: 'matches',
|
||||
files: [{ path: 'a.ts', matches: [{ lineNumber: 1, line: 'one' }, { lineNumber: 2, line: 'two' }] }],
|
||||
truncated: true,
|
||||
total: 3,
|
||||
@@ -825,11 +825,10 @@ describe('presentation', () => {
|
||||
const view = presentGrepResult({ pattern: 'e' }, result)
|
||||
expect(view).toEqual({
|
||||
card: 'search',
|
||||
kind: 'matches',
|
||||
shape: 'matches',
|
||||
files: [{ path: 'a.ts', matches: [{ lineNumber: 1, line: 'one' }, { lineNumber: 2, line: 'two' }] }],
|
||||
truncated: true,
|
||||
total: 3,
|
||||
content: result.content,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -838,9 +837,9 @@ describe('presentation', () => {
|
||||
bash.handler = () => runResult('a.ts\nb.ts\nc.ts\n')
|
||||
const result = await call(ctx, 'glob', { pattern: '*.ts' }, { agent: agent('/w') })
|
||||
if (result.isError) throw new Error('expected glob success')
|
||||
expect(result.meta).toEqual({ kind: 'paths', paths: ['a.ts', 'b.ts'], truncated: true, total: 3 })
|
||||
expect(result.meta).toEqual({ shape: 'paths', paths: ['a.ts', 'b.ts'], truncated: true, total: 3 })
|
||||
const view = presentGlobResult({ pattern: '*.ts' }, result)
|
||||
expect(view).toEqual({ card: 'search', kind: 'paths', paths: ['a.ts', 'b.ts'], truncated: true, total: 3, content: result.content })
|
||||
expect(view).toEqual({ card: 'search', shape: 'paths', paths: ['a.ts', 'b.ts'], truncated: true, total: 3 })
|
||||
})
|
||||
|
||||
it('nested Code dispatch computes no meta, so presentResult falls back to the generic card', async () => {
|
||||
@@ -860,15 +859,15 @@ describe('presentation', () => {
|
||||
expect(presentGrepResult({ pattern: 'x' }, errorResult)).toBeUndefined()
|
||||
expect(presentGlobResult({ pattern: '*' }, errorResult)).toBeUndefined()
|
||||
// A grep result carrying a paths-shaped meta (and vice versa) is not this
|
||||
// tool's shape: each presenter narrows to its own kind and otherwise falls back.
|
||||
const pathsResult = { content: [], isError: false, meta: { kind: 'paths', paths: ['a.ts'], truncated: false, total: 1 } }
|
||||
const matchesResult = { content: [], isError: false, meta: { kind: 'matches', files: [], truncated: false, total: 0 } }
|
||||
// tool's shape: each presenter narrows to its own shape and otherwise falls back.
|
||||
const pathsResult = { content: [], isError: false, meta: { shape: 'paths', paths: ['a.ts'], truncated: false, total: 1 } }
|
||||
const matchesResult = { content: [], isError: false, meta: { shape: 'matches', files: [], truncated: false, total: 0 } }
|
||||
expect(presentGrepResult({ pattern: 'x' }, pathsResult)).toBeUndefined()
|
||||
expect(presentGlobResult({ pattern: '*' }, matchesResult)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('presentResult falls back to the generic card on malformed replayed meta', () => {
|
||||
const malformed = { content: [], isError: false, meta: { kind: 'matches', files: 'nope', truncated: false, total: 0 } }
|
||||
const malformed = { content: [], isError: false, meta: { shape: 'matches', files: 'nope', truncated: false, total: 0 } }
|
||||
expect(presentGrepResult({ pattern: 'x' }, malformed)).toBeUndefined()
|
||||
expect(presentGlobResult({ pattern: '*' }, { content: [], isError: false, meta: 42 })).toBeUndefined()
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user