fix(web): address the second review round on context provenance
- An empty replacement catalog is a real catalog: `renderCatalogUpdate()` publishes zero entries when the last skill disappears, and falling back would hide that every earlier name was retired. - The opaque fallback keeps a `form` declaration this version cannot present. It is the one place a newer or foreign log's declared shape would otherwise vanish from the UI entirely, since the row marker is also absent there. - An instruction change with an unrecognized `action` disqualifies the record. The action decides the word the row shows, so an unknown one would be presented as loaded or updated. - The catalog list bounds itself and reports the withheld count. Entry count is unbounded and the scrollport bounds height, not node count. - A catalog message keeps content blocks this version does not know, instead of dropping model-visible content the extensible union may carry. - `core.md` defines `ContextFormed`, the interface actually carrying the optional field, beside `ContextForm`. - The superseded-in-part bullet states the affected clauses as one rule rather than enumerating them; two rounds of enumeration each missed some, which is the shape being fragile rather than the list being wrong. - The note records the one migration case that does not self-heal: an old-format catalog as the only one, with an empty current view, leaves a stale catalog nothing replaces.
This commit is contained in:
creatixchu
@@ -13,6 +13,9 @@ import css from './ContextBody.module.css'
|
||||
/** Model-facing text stays bounded at the disclosure, not at the producer. */
|
||||
const MAX_CHARS = 20_000
|
||||
|
||||
/** Rows a list body materializes before summarizing the remainder. */
|
||||
const MAX_ENTRIES = 200
|
||||
|
||||
type Translate = ChatViewSlotProps['t']
|
||||
|
||||
/** One durable source narrowed to the readable-record shape; null for anything else. */
|
||||
@@ -61,14 +64,22 @@ function fieldValue(value: unknown, t: Translate): string {
|
||||
}
|
||||
|
||||
/**
|
||||
* Provenance fields as a key/value list. `kind` is omitted because the row
|
||||
* header already names the producer, and `form` because the presentation the
|
||||
* reader is looking at IS that value.
|
||||
* Provenance fields as a key/value list. `kind` is always omitted because the
|
||||
* row header already names the producer. `form` is omitted only when a
|
||||
* dedicated body rendered for it — then the presentation the reader is looking
|
||||
* at IS that value. On the opaque fallback the declaration is kept, because
|
||||
* that is the one place a form this version cannot present would otherwise
|
||||
* disappear from the UI entirely.
|
||||
*/
|
||||
function SourceFields({ source, t }: { source: unknown; t: Translate }): ReactNode {
|
||||
function SourceFields({ source, formRendered, t }: {
|
||||
source: unknown
|
||||
formRendered: boolean
|
||||
t: Translate
|
||||
}): ReactNode {
|
||||
const record = asRecord(source)
|
||||
if (record === null) return null
|
||||
const rows = Object.entries(record).filter(([key]) => key !== 'kind' && key !== 'form')
|
||||
const hidden = formRendered ? ['kind', 'form'] : ['kind']
|
||||
const rows = Object.entries(record).filter(([key]) => !hidden.includes(key))
|
||||
if (rows.length === 0) return null
|
||||
return (
|
||||
<dl className={css.fields} data-context-fields>
|
||||
@@ -82,6 +93,28 @@ function SourceFields({ source, t }: { source: unknown; t: Translate }): ReactNo
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Content blocks this UI version does not know, kept visible rather than
|
||||
* dropped: the block union is merge-extensible, so a newer or foreign log may
|
||||
* carry a shape this build has no presentation for.
|
||||
* @param props - The unrecognized blocks and the locale seat.
|
||||
* @returns One generic JSON block per unknown entry.
|
||||
*/
|
||||
function UnknownBlocks({ blocks, t }: { blocks: readonly unknown[]; t: Translate }): ReactNode {
|
||||
return (
|
||||
<>
|
||||
{blocks.map((block, index) => (
|
||||
<JsonBlock
|
||||
key={index}
|
||||
label={t('message.unknownBlock')}
|
||||
payload={block}
|
||||
truncatedLabel={total => t('json.truncated', { total })}
|
||||
/>
|
||||
))}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The model-facing content of one context, shared by every form that shows it:
|
||||
* the text with its real line breaks, then any block this UI version does not
|
||||
@@ -97,14 +130,7 @@ function ModelFacingContent({ content, t }: {
|
||||
return (
|
||||
<>
|
||||
{text !== '' && <pre className={css.text} data-context-text>{boundedText(text, t)}</pre>}
|
||||
{rest.map((block, index) => (
|
||||
<JsonBlock
|
||||
key={index}
|
||||
label={t('message.unknownBlock')}
|
||||
payload={block}
|
||||
truncatedLabel={total => t('json.truncated', { total })}
|
||||
/>
|
||||
))}
|
||||
<UnknownBlocks blocks={rest} t={t} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
@@ -124,14 +150,14 @@ export function OpaqueBody({ content, source, t }: {
|
||||
return (
|
||||
<>
|
||||
<ModelFacingContent content={content} t={t} />
|
||||
<SourceFields source={source} t={t} />
|
||||
<SourceFields source={source} formRendered={false} t={t} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
/** One reconciled instruction file, as the durable source records it. */
|
||||
interface InstructionChange {
|
||||
action: string
|
||||
action: 'set' | 'replace' | 'remove'
|
||||
path: string
|
||||
digest?: string
|
||||
}
|
||||
@@ -157,14 +183,13 @@ function instructionChanges(source: unknown): InstructionChange[] | null {
|
||||
const path = change['path']
|
||||
if (typeof path !== 'string' || path === '') return null
|
||||
const action = change['action']
|
||||
// The action decides which word the row shows, so an unrecognized one is
|
||||
// not a readable change — it would be presented as loaded or updated.
|
||||
if (action !== 'set' && action !== 'replace' && action !== 'remove') return null
|
||||
const digest = change['digest']
|
||||
if (seen.has(path)) continue
|
||||
seen.add(path)
|
||||
changes.push({
|
||||
action: typeof action === 'string' ? action : '',
|
||||
path,
|
||||
...typeof digest === 'string' ? { digest } : {},
|
||||
})
|
||||
changes.push({ action, path, ...typeof digest === 'string' ? { digest } : {} })
|
||||
}
|
||||
return changes.length === 0 ? null : changes
|
||||
}
|
||||
@@ -228,7 +253,9 @@ function catalogEntries(source: unknown): CatalogEntry[] | null {
|
||||
if (typeof name !== 'string' || name === '' || typeof description !== 'string') return null
|
||||
entries.push({ name, description })
|
||||
}
|
||||
return entries.length === 0 ? null : entries
|
||||
// An empty list is a real catalog: a replacement with no entries retires
|
||||
// every earlier name. Only an unreadable shape falls back.
|
||||
return entries
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -249,11 +276,15 @@ export function CatalogBody({ content, source, t }: {
|
||||
const entries = catalogEntries(source)
|
||||
if (entries === null) return <OpaqueBody content={content} source={source} t={t} />
|
||||
const update = asRecord(source)?.['update'] === true
|
||||
// Entry count is unbounded (a provider may publish any number of skills), and
|
||||
// the scrollport bounds height, not node count — so the list bounds itself.
|
||||
const shown = entries.slice(0, MAX_ENTRIES)
|
||||
const { rest } = partitionContent(content)
|
||||
return (
|
||||
<>
|
||||
{update && <p className={css.catalogNotice} data-context-catalog-update>{t('message.context.catalog.replaced')}</p>}
|
||||
<ul className={css.entries} data-context-entries>
|
||||
{entries.map((entry, index) => (
|
||||
{shown.map((entry, index) => (
|
||||
// Index key: a hand-edited or foreign log may repeat a name, and a
|
||||
// duplicate React key would drop a row the model did see.
|
||||
<li key={index} className={css.entry}>
|
||||
@@ -262,6 +293,14 @@ export function CatalogBody({ content, source, t }: {
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
{shown.length < entries.length && (
|
||||
<p className={css.catalogNotice} data-context-entries-truncated>
|
||||
{t('message.context.catalog.more', { count: entries.length - shown.length })}
|
||||
</p>
|
||||
)}
|
||||
{/* The block union is merge-extensible: a catalog message carrying an
|
||||
unknown block still shows it rather than dropping model-visible content. */}
|
||||
<UnknownBlocks blocks={rest} t={t} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -59,6 +59,7 @@ export const zh = {
|
||||
'message.context.instructions.updated': '已更新',
|
||||
'message.context.instructions.removed': '已移除',
|
||||
'message.context.catalog.replaced': '替换目录',
|
||||
'message.context.catalog.more': '…还有 {count} 条',
|
||||
'message.steering': '插话',
|
||||
'message.compaction': '上下文已压缩',
|
||||
'message.compaction.expand': '点击查看压缩摘要',
|
||||
@@ -178,6 +179,7 @@ export const en = {
|
||||
'message.context.instructions.updated': 'updated',
|
||||
'message.context.instructions.removed': 'removed',
|
||||
'message.context.catalog.replaced': 'Replacement catalog',
|
||||
'message.context.catalog.more': '… {count} more',
|
||||
'message.steering': 'Interjection',
|
||||
'message.compaction': 'Context compacted',
|
||||
'message.compaction.expand': 'View compaction summary',
|
||||
|
||||
@@ -411,13 +411,34 @@ describe('MessageItem arms', () => {
|
||||
.toMatch(/… 已截断，共 \d+ 字符$/)
|
||||
})
|
||||
|
||||
it('a catalog whose source carries no entries falls back to the opaque body', () => {
|
||||
it('an empty replacement catalog stays a catalog: it retires every earlier name', () => {
|
||||
// `renderCatalogUpdate` legitimately publishes zero entries when the last
|
||||
// skill disappears; falling back would hide that the catalog was cleared.
|
||||
const view = render(
|
||||
<MessageItem t={t} node={{
|
||||
kind: 'context',
|
||||
seq: 3,
|
||||
content: [{ type: 'text', text: 'catalog prose' }],
|
||||
source: { kind: 'skill-catalog', form: 'catalog', entries: [] },
|
||||
source: { kind: 'skill-catalog', form: 'catalog', update: true, entries: [] },
|
||||
provenance: { role: 'inject', label: 'skill-catalog' },
|
||||
form: 'catalog',
|
||||
} as never}
|
||||
/>,
|
||||
)
|
||||
fireEvent.click(view.getByRole('button', { name: /^上下文注入\s*skill-catalog$/ }))
|
||||
expect(view.container.querySelector('[data-context-catalog-update]')?.textContent).toBe('替换目录')
|
||||
expect(view.container.querySelectorAll('[data-context-entries] li')).toHaveLength(0)
|
||||
expect(view.container.querySelector('[data-context-injection-body]')?.getAttribute('data-context-form'))
|
||||
.toBe('catalog')
|
||||
})
|
||||
|
||||
it('a catalog whose entries are unreadable falls back to the opaque body', () => {
|
||||
const view = render(
|
||||
<MessageItem t={t} node={{
|
||||
kind: 'context',
|
||||
seq: 3,
|
||||
content: [{ type: 'text', text: 'catalog prose' }],
|
||||
source: { kind: 'skill-catalog', form: 'catalog', entries: 'not-a-list' },
|
||||
provenance: { role: 'inject', label: 'skill-catalog' },
|
||||
form: 'catalog',
|
||||
} as never}
|
||||
@@ -428,51 +449,71 @@ describe('MessageItem arms', () => {
|
||||
expect(view.container.querySelector('[data-context-text]')?.textContent).toBe('catalog prose')
|
||||
})
|
||||
|
||||
it('a recalled session titles its row by role and names the sessions it read', () => {
|
||||
it('bounds a large catalog and says how many rows it withheld', () => {
|
||||
const entries = Array.from({ length: 205 }, (_, index) => ({ name: `s-${index}`, description: 'd' }))
|
||||
const view = render(
|
||||
<MessageItem t={t} node={{
|
||||
kind: 'context', seq: 3, content: [{ type: 'text', text: 'catalog prose' }],
|
||||
source: { kind: 'skill-catalog', form: 'catalog', entries },
|
||||
provenance: { role: 'inject', label: 'skill-catalog' },
|
||||
form: 'catalog',
|
||||
} as never}
|
||||
/>,
|
||||
)
|
||||
fireEvent.click(view.getByRole('button', { name: /^上下文注入\s*skill-catalog$/ }))
|
||||
expect(view.container.querySelectorAll('[data-context-entries] li')).toHaveLength(200)
|
||||
expect(view.container.querySelector('[data-context-entries-truncated]')?.textContent).toBe('…还有 5 条')
|
||||
})
|
||||
|
||||
it('a catalog keeps a content block this version does not know', () => {
|
||||
const view = render(
|
||||
<MessageItem t={t} node={{
|
||||
kind: 'context',
|
||||
seq: 3,
|
||||
content: [{ type: 'text', text: 'snapshot' }],
|
||||
source: { kind: 'session-reference', version: 1, references: [{ label: '重构 loader' }] },
|
||||
provenance: { role: 'recall', label: '重构 loader' },
|
||||
form: null,
|
||||
content: [{ type: 'text', text: 'prose' }, { type: 'future-block', payload: 1 }],
|
||||
source: { kind: 'skill-catalog', form: 'catalog', entries: [{ name: 'a', description: 'b' }] },
|
||||
provenance: { role: 'inject', label: 'skill-catalog' },
|
||||
form: 'catalog',
|
||||
} as never}
|
||||
/>,
|
||||
)
|
||||
expect(view.getByRole('button', { name: /^跨会话召回\s*重构 loader$/ })).toBeTruthy()
|
||||
expect(view.queryByText('上下文注入')).toBeNull()
|
||||
fireEvent.click(view.getByRole('button', { name: /^上下文注入\s*skill-catalog$/ }))
|
||||
expect(view.getByText(/未知内容块/)).toBeTruthy()
|
||||
})
|
||||
|
||||
it('keeps the producer name visible while the context body is expanded', () => {
|
||||
it('an instruction change with an unrecognized action falls back whole', () => {
|
||||
// The action decides the word the row shows, so an unknown one cannot be
|
||||
// presented as loaded or updated.
|
||||
const view = render(
|
||||
<MessageItem t={t} node={{
|
||||
kind: 'context',
|
||||
seq: 3,
|
||||
content: [{ type: 'text', text: 'instructions' }],
|
||||
source: { kind: 'workspace-instructions', changes: [{ path: 'AGENTS.md' }] },
|
||||
provenance: { role: 'inject', label: 'AGENTS.md' },
|
||||
form: null,
|
||||
content: [{ type: 'text', text: 'instruction prose' }],
|
||||
source: { kind: 'workspace-instructions', form: 'instructions', changes: [{ action: 'merge', path: 'A.md' }] },
|
||||
provenance: { role: 'inject', label: 'workspace-instructions' },
|
||||
form: 'instructions',
|
||||
} as never}
|
||||
/>,
|
||||
)
|
||||
const disclosure = view.getByRole('button', { name: /^上下文注入\s*AGENTS\.md$/ })
|
||||
fireEvent.click(disclosure)
|
||||
expect(disclosure.getAttribute('aria-expanded')).toBe('true')
|
||||
expect(view.container.querySelector('[data-context-source]')?.textContent).toBe('AGENTS.md')
|
||||
fireEvent.click(view.getByRole('button', { name: /^上下文注入\s*workspace-instructions$/ }))
|
||||
expect(view.container.querySelector('[data-context-files]')).toBeNull()
|
||||
expect(view.container.querySelector('[data-context-text]')?.textContent).toBe('instruction prose')
|
||||
})
|
||||
|
||||
it('a context source that names no producer shows the role alone', () => {
|
||||
it('the opaque fallback keeps a form declaration this version cannot present', () => {
|
||||
// Otherwise a newer or foreign log's declared shape vanishes from the UI.
|
||||
const view = render(
|
||||
<MessageItem t={t} node={{
|
||||
kind: 'context', seq: 3, content: [{ type: 'text', text: 'x' }], source: null,
|
||||
provenance: { role: 'inject', label: null },
|
||||
kind: 'context', seq: 3, content: [{ type: 'text', text: 'x' }],
|
||||
source: { kind: 'plugin', plugin: 'later', form: 'a-later-form' },
|
||||
provenance: { role: 'inject', label: 'later' },
|
||||
form: null,
|
||||
} as never}
|
||||
/>,
|
||||
)
|
||||
expect(view.getByRole('button', { name: '上下文注入' })).toBeTruthy()
|
||||
expect(view.container.querySelector('[data-context-source]')).toBeNull()
|
||||
fireEvent.click(view.getByRole('button', { name: /^上下文注入\s*later$/ }))
|
||||
const fields = [...view.container.querySelectorAll('[data-context-fields] dt')].map(node => node.textContent)
|
||||
expect(fields).toEqual(['plugin', 'form'])
|
||||
})
|
||||
|
||||
it('unknown nodes retain the generic JSON row', () => {
|
||||
|
||||
Reference in New Issue
Block a user