fix: address concrete terminology review findings
This commit is contained in:
1 parent
71d50b4a84
commit
2b05a43eca
10 files changed
+48
-19
No files matched your search
@@ -119,7 +119,7 @@ Real-API tests and demos read `DEEPSEEK_API_KEY`, optional `DEEPSEEK_BASE_URL`,
|
||||
- **Trust TypeScript at typed same-process boundaries.** Do not add runtime validation, fallback behavior, or hostile-input tests solely for values the static interface requires; validate at parser/config, queued, model/tool JSON, durable/file, worker, process, and wire boundaries.
|
||||
- **Source plane vs artifact plane, never mixed.** Static gates and tests resolve workspace imports through tsconfig `paths` to `src` and pass on a clean tree; gates consuming built `lib/` declare that dependency ([layout](docs/development.md#typescript-project-layout)).
|
||||
- **Keep compiler faces explicit.** A package with both Host and Client programs exposes face-specific leaf configs and a solution-only root; repo-wide programs seed a face config, never the root solution ([layout](docs/development.md#typescript-project-layout)).
|
||||
- **An empty `catch` names the swallowed error** and why; keep its `try` to one statement.
|
||||
- **An empty `catch` names the error** and why; keep its `try` to one statement.
|
||||
- **Keep comments local.** Do not restate code, expand unrelated comments, or explain distant behavior without local need ([rationale](.agents/notes/implemented/process/2026-08-09-concrete-prose-names-actors-and-recorded-facts.md)).
|
||||
- **Ban `prove` + `nance`** ([rule](.agents/notes/implemented/process/2026-08-26-ban-ambiguous-origin-label.md)).
|
||||
- **Prefer symmetry for parallel values**; unexplained asymmetry usually signals a missed extraction.
|
||||
|
||||
@@ -73,7 +73,7 @@ export interface AssistantMessageNode {
|
||||
step: number
|
||||
blocks: readonly AssistantBlock[]
|
||||
usage?: unknown
|
||||
providerInfo?: AssistantProviderMetadataView
|
||||
providerMetadata?: AssistantProviderMetadataView
|
||||
requestConfig?: AssistantRequestConfig
|
||||
/** Timing derived from the recorded step/chunk/message event sequence. */
|
||||
timing?: AssistantTiming
|
||||
|
||||
@@ -133,7 +133,7 @@ interface RequestViewBase {
|
||||
error?: string
|
||||
/** Stable provider code for localized presentation of known failures. */
|
||||
errorCode?: string
|
||||
providerInfo?: AssistantProviderMetadataView
|
||||
providerMetadata?: AssistantProviderMetadataView
|
||||
requestConfig?: AssistantRequestConfig
|
||||
usage?: unknown
|
||||
/** Assistant message or compaction summary sequence produced by this request. */
|
||||
|
||||
@@ -250,8 +250,8 @@ export function TrajectoryView({
|
||||
const turn = request?.turn ?? node?.turn
|
||||
const step = request?.step ?? node?.step
|
||||
if (turn === undefined || step === undefined) continue
|
||||
const provider = request?.providerInfo?.provider ?? node?.providerInfo?.provider
|
||||
const model = request?.providerInfo?.model ?? node?.providerInfo?.model
|
||||
const provider = request?.providerMetadata?.provider ?? node?.providerMetadata?.provider
|
||||
const model = request?.providerMetadata?.model ?? node?.providerMetadata?.model
|
||||
const requestConfig = request?.requestConfig ?? node?.requestConfig
|
||||
numbered.push({
|
||||
seq: entry.seq,
|
||||
@@ -292,12 +292,12 @@ export function TrajectoryView({
|
||||
...(request.error === undefined ? {} : { error: request.error }),
|
||||
...(request.errorCode === undefined ? {} : { errorCode: request.errorCode }),
|
||||
resultSeq: request.startSeq,
|
||||
...(request.providerInfo?.provider === undefined
|
||||
...(request.providerMetadata?.provider === undefined
|
||||
? {}
|
||||
: { provider: request.providerInfo.provider }),
|
||||
...(request.providerInfo?.model === undefined
|
||||
: { provider: request.providerMetadata.provider }),
|
||||
...(request.providerMetadata?.model === undefined
|
||||
? {}
|
||||
: { model: request.providerInfo.model }),
|
||||
: { model: request.providerMetadata.model }),
|
||||
...(request.requestConfig === undefined ? {} : { requestConfig: request.requestConfig }),
|
||||
...(usage === undefined ? {} : { usage }),
|
||||
...(cumulativeUsage === undefined ? {} : { cumulativeUsage }),
|
||||
|
||||
@@ -252,7 +252,7 @@ function finalNode(
|
||||
step: state.step,
|
||||
blocks: toAssistantBlocks(event.data.message.content),
|
||||
usage: event.data.usage,
|
||||
providerInfo: {
|
||||
providerMetadata: {
|
||||
provider: event.data.message.source.provider,
|
||||
model: event.data.message.source.model,
|
||||
},
|
||||
@@ -309,7 +309,7 @@ function assistantRequest(
|
||||
? {}
|
||||
: {
|
||||
resultSeq: node.seq,
|
||||
...(node.providerInfo === undefined ? {} : { providerInfo: node.providerInfo }),
|
||||
...(node.providerMetadata === undefined ? {} : { providerMetadata: node.providerMetadata }),
|
||||
}),
|
||||
...(state.usage === undefined ? {} : { usage: state.usage }),
|
||||
}
|
||||
|
||||
@@ -64,7 +64,7 @@ function requestFromState(
|
||||
resultSeq: summary.seq,
|
||||
summary: summary.data.summary,
|
||||
...(summary.data.rawOutput === undefined ? {} : { rawOutput: summary.data.rawOutput }),
|
||||
providerInfo: { provider: summary.data.provider, model: summary.data.model },
|
||||
providerMetadata: { provider: summary.data.provider, model: summary.data.model },
|
||||
requestConfig: {
|
||||
provider: summary.data.provider,
|
||||
model: summary.data.model,
|
||||
|
||||
@@ -397,7 +397,7 @@ describe('Trajectory conversation Definitions', () => {
|
||||
purpose: 'assistant',
|
||||
resultSeq: 3,
|
||||
status: 'error',
|
||||
providerInfo: { provider: 'test', model: 'test' },
|
||||
providerMetadata: { provider: 'test', model: 'test' },
|
||||
}])
|
||||
})
|
||||
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
import { mkdtempSync, rmSync, symlinkSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { findConcreteTermViolations } from './verify-concrete-terms.ts'
|
||||
import { findConcreteTermViolations, readTrackedSource } from './verify-concrete-terms.ts'
|
||||
|
||||
const blockedTerm = 'prove' + 'nance'
|
||||
|
||||
@@ -21,6 +24,27 @@ describe('concrete terminology policy', () => {
|
||||
.toEqual([{ file: 'packages/example/src/source.ts', line: 1 }])
|
||||
})
|
||||
|
||||
it('normalizes compatibility characters before scanning', () => {
|
||||
const fullwidthTerm = blockedTerm.split('')
|
||||
.map(character => String.fromCodePoint(character.charCodeAt(0) + 0xfee0))
|
||||
.join('')
|
||||
expect(findConcreteTermViolations('packages/example/src/source.ts', fullwidthTerm))
|
||||
.toEqual([{ file: 'packages/example/src/source.ts', line: 1 }])
|
||||
})
|
||||
|
||||
it.skipIf(process.platform === 'win32')('reads the target of a dangling tracked symlink', () => {
|
||||
const repoRoot = mkdtempSync(join(tmpdir(), 'dsh-concrete-terms-'))
|
||||
try {
|
||||
symlinkSync(`../${blockedTerm}-target`, join(repoRoot, 'tracked-link'))
|
||||
expect(findConcreteTermViolations(
|
||||
'tracked-link',
|
||||
readTrackedSource(repoRoot, 'tracked-link') ?? '',
|
||||
)).toEqual([{ file: 'tracked-link', line: 1 }])
|
||||
} finally {
|
||||
rmSync(repoRoot, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('accepts exact replacement terms', () => {
|
||||
expect(findConcreteTermViolations(
|
||||
'packages/example/src/origin.ts',
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
/** Reject one ambiguous origin label from maintained tracked files. */
|
||||
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { existsSync, lstatSync, readFileSync, readlinkSync } from 'node:fs'
|
||||
import { lstatSync, readFileSync, readlinkSync } from 'node:fs'
|
||||
import { resolve } from 'node:path'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
|
||||
@@ -55,10 +55,16 @@ function trackedFiles(repoRoot: string): string[] {
|
||||
return files
|
||||
}
|
||||
|
||||
function readTrackedSource(repoRoot: string, file: string): string | undefined {
|
||||
/**
|
||||
* Read one tracked file without following a symlink to its target.
|
||||
* @param repoRoot - Repository root containing the tracked path.
|
||||
* @param file - Repository-relative tracked path.
|
||||
* @returns File text, the symlink target, or undefined when the path is absent or not a file.
|
||||
*/
|
||||
export function readTrackedSource(repoRoot: string, file: string): string | undefined {
|
||||
const path = resolve(repoRoot, file)
|
||||
if (!existsSync(path)) return undefined
|
||||
const stat = lstatSync(path)
|
||||
const stat = lstatSync(path, { throwIfNoEntry: false })
|
||||
if (stat === undefined) return undefined
|
||||
if (stat.isSymbolicLink()) return readlinkSync(path)
|
||||
return stat.isFile() ? readFileSync(path, 'utf8') : undefined
|
||||
}
|
||||
|
||||
@@ -243,7 +243,6 @@ export default defineConfig({
|
||||
'packages/client/ui-chat/src/client/conversation-nodes/*',
|
||||
'packages/client/ui-chat/src/client/details/*',
|
||||
'packages/client/ui-chat/src/client/model/*',
|
||||
'packages/client/ui-chat/src/client/contract/context-producer.ts',
|
||||
'packages/client/ui-chat/src/client/contract/snapshot.ts',
|
||||
'packages/client/ui-chat/src/client/historical-images.ts',
|
||||
'packages/client/ui-primitives/src/DisclosureRow.tsx',
|
||||
|
||||
Reference in new issue
Block a user