From 185f975daab0657060553003ae0e0b7d19d814ba Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Thu, 10 Sep 2026 01:41:53 -0700 Subject: [PATCH 1/2] fix(ask): keep every bought answer when a cited run is interrupted Ctrl-C on a run whose answers carry `trace://` citations discarded every answer. `verifyCitations` sat outside `askOne`'s try/catch and forwarded the run's AbortSignal to `store.hasSpans`, which starts with `throwIfAborted()`. Once aborted, `askOne` rejected, the worker pool rejected, `runTraceQuestions` threw, and `cmdAsk` never reached `writeTraceQuestionsArtifacts`: no `answers.json`, no `report.md`, and the answers already paid for were lost. That is the one path `cmdAsk` installs a SIGINT handler to serve, and it contradicted both the docs and the command's own guarantee. - Citation checking takes no signal. The index is built before the first question runs, so it is an in-memory lookup and need not be cancellable. - The whole post-engine block (citations plus the schema parse) runs inside its own guard that records `aborted` or `error` on that answer, so no later check can throw an answer away either. - The worker pool no longer lets an exception escape with the answers: any unfilled question is recorded with the cause and the run still returns, so `cmdAsk` writes both artifacts and decides the exit code from them. Also, from the same review: - `budget-refused` is decided from the ledger, not only from the error text. The refusal happens behind the DSPy bridge, whose HTTP error handling can replace the Node message, so a failure is reported as `budget-refused` whenever settled spend left less than one call's reservation. - `totals.wallTimeMs` now starts before the trace file is written and indexed, with `totals.setupTimeMs` naming that part; wall time is the metric this command exists to improve. - `result.effectiveConcurrency` reports the workers actually created, next to the requested limit, instead of only the request. - Default question IDs are assigned after explicit ones are reserved, so a file entry named `q2` plus a positional question no longer kills the run. - `analystRunDetail` bounds the whole cell, so appending a rejection summary cannot push it past the length `ANALYST_DETAIL_MAX_CHARS` names. - An answer's own Markdown headings are demoted below its question's section, so an answer cannot reflow the report's outline. - README, docs/trace-analysts.md, and both shipped skills name `ask`, the wider exit-code scope (`investigate`, `improve`, and config-declared analyzers), and the interrupt guarantee. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 6 +- docs/trace-analysts.md | 12 +- skills/build-trace-analyst/SKILL.md | 3 + skills/inspect-agent-traces/SKILL.md | 42 +++--- src/ask.ts | 214 ++++++++++++++++++++++----- src/cli.ts | 4 +- src/report.ts | 22 ++- tests/ask.test.ts | 128 +++++++++++++++- tests/report.test.ts | 16 ++ 9 files changed, 381 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 6f69670..1003edd 100644 --- a/README.md +++ b/README.md @@ -294,9 +294,10 @@ See [Replay verification](./docs/replay-verify.md) for setup, semantics, and hon | `--since ` | `upload`: window, `30m`/`2h`/`7d` or ISO (default 24h); `analyze`: ISO cutoff | | `--out ` | Write the report to a file | | `--dir ` | `improve`: write the full artifact pack to this directory; `ask`: write `answers.json` + `report.md` there | -| `--otlp ` | **READ** OTLP-JSONL from any system, skipping the adapters; a directory reads the OTLP files under it (only `otlp/` when the producer made one) and names the JSONL that is not OTLP. `validate`, `analyze`, `investigate`, `improve`, `stream` | +| `--otlp ` | **READ** OTLP-JSONL from any system, skipping the adapters; a directory reads the OTLP files under it (only `otlp/` when the producer made one) and names the JSONL that is not OTLP. `validate`, `analyze`, `investigate`, `improve`, `ask`, `stream` | | `--otlp-out ` | **WRITE** the OTLP artifact here (also evidence provenance / dry-run upload preview) | | `--format ` | File `analyze`, `export`, or `stream`: `auto`, `policy-evidence`, `sandbox-events`, `openinference`, `intelligence-spans`, or `chat-trajectory` | +| `--source-bundle ` | `analyze` / `investigate` / `improve` / `ask`: read a retained full bundle and explicitly grant source-field reads | | `--llm` / `--budget ` | Enable agentic analysts (needs `TANGLE_API_KEY` + Python with `agent-eval-rpc[dspy]`) / cap their spend | | `--question ` | `ask`: one question, repeatable. Kept short so the engine sees it whole | | `--questions ` | `ask`: JSON array of questions — strings, or `{ id?, question, instructions?, answerSchema? }` | @@ -495,7 +496,8 @@ What it checks, and what it costs: - **Citations.** Every `trace:///span/` URI in an answer is looked up in the trace. An answer that cites a span the trace does not hold fails. - **Budget.** `--budget` is one ceiling shared by every question. `--question-budget` bounds one question. The run refuses to start when the budget cannot cover a single model call, and warns when the budget admits fewer concurrent calls than `--concurrency`. - **Cost.** Each cost carries its provenance: `observed` from a provider receipt, `estimated` from token counts, or `uncaptured`. An uncaptured cost stays null; it never becomes zero. -- **Exit code.** `ask` writes both artifacts first, then exits 1 when any question failed, returned no answer, broke its schema, or cited a span that does not exist. A failed question never costs the other answers. +- **Exit code.** `ask` writes both artifacts first, then exits 1 when any question failed, returned no answer, broke its schema, or cited a span that does not exist. A failed question never costs the other answers, and neither does Ctrl-C: the answers already bought are recorded, the questions the run never reached are recorded as `aborted`, and both artifacts are still written. +- **Wall time.** `totals.wallTimeMs` covers the whole run, including writing and indexing the trace file; `totals.setupTimeMs` says how much of it that setup was. `totals.peakConcurrency` is how many questions actually overlapped, which is at most `min(--concurrency, questions)`. `ask` uses the same engine and credentials as `--llm`, so it needs `TANGLE_API_KEY` and a Python interpreter with `agent-eval-rpc[dspy]`. Do not pass `--llm`; the command is model-backed by definition. diff --git a/docs/trace-analysts.md b/docs/trace-analysts.md index 01464eb..810b682 100644 --- a/docs/trace-analysts.md +++ b/docs/trace-analysts.md @@ -117,7 +117,9 @@ This package carries no JSON Schema library, and a constraint that is quietly ig - Every `trace:///span/` URI in an answer is resolved against the store. An unresolvable citation fails that question. - Findings the answer submits still pass the same evidence gate as the built-in kinds. Refused findings are counted by reason in both artifacts. - A failed question never stops the others. Its failure is recorded on its own answer, and the remaining answers are written. +- Ctrl-C keeps what the run already bought. The signal reaches the engine, not the checks that follow it: an answer that came back is kept with its citations resolved, and each question the run never reached is recorded as `aborted`. - The artifacts are written before the exit code is decided. `ask` exits 1 when any question failed, returned no answer, broke its schema, or cited a missing span. +- `totals.wallTimeMs` covers the whole run, including writing and indexing the trace file; `totals.setupTimeMs` names that part. `result.effectiveConcurrency` is the number of workers the run created, `min(--concurrency, questions)`, and `totals.peakConcurrency` is how many actually overlapped. ### Budget under concurrency @@ -129,6 +131,10 @@ Two consequences follow. - A budget that admits fewer concurrent reservations than `--concurrency` still runs, and the report carries a warning naming how many concurrent calls it covers. A question the ledger refuses is reported as `budget-refused`, and the answers already produced are kept. +That kind is decided from the accounting, not only from the error text. +The refusal happens inside the model proxy, behind the DSPy bridge, whose HTTP error handling can replace the Node error with its own message. +So a failed question is reported as `budget-refused` whenever the shared ledger's settled spend left less than one model call's reservation at the time it failed, whatever the message says. +The message itself is kept verbatim on the answer. ### SDK @@ -168,9 +174,13 @@ The common reasons are an excerpt the cited span does not contain, a span the tr ## External analyzer failures exit non-zero `--analyzer halo|hodoscope|prime|` promises that engine's output. -An analyzer that fails now writes its error into the report as before, and then `analyze` exits 1 naming every analyzer that failed. +An analyzer that fails now writes its error into the report as before, and then the command exits 1 naming every analyzer that failed. Scripts that treated exit 0 as "the analyzer ran" were reading a report that said otherwise. +The check runs on `analyze`, `investigate`, and `improve`, and it covers every analyzer the run requested, not only the ones named on the command line. +`investigate` and `improve` load the default traces config file, so an analyzer declared in `externalAnalyzers` there is a requested analyzer too, and a flaky one now turns those two commands red. +`analyze` does not load a default config, so only its own `--analyzer` flags reach the check. + ## Codex tool outcomes Both function and custom tool outputs use the same status parser. diff --git a/skills/build-trace-analyst/SKILL.md b/skills/build-trace-analyst/SKILL.md index 26d837f..1935e2a 100644 --- a/skills/build-trace-analyst/SKILL.md +++ b/skills/build-trace-analyst/SKILL.md @@ -17,6 +17,9 @@ Check live analysts separately only for `traces stream`. Extend an existing analyst when it already emits the target with usable evidence. Do not inspect bundled `dist` or `node_modules`. +Answer a one-off question with `traces ask --question ""` instead. +Build an analyst when the question repeats and its answer must be scored, not read. + ## Implement - Use `Analyst`, `AnalystRegistry`, `TraceAnalysisStore`, and `makeFinding` from `@tangle-network/traces`. diff --git a/skills/inspect-agent-traces/SKILL.md b/skills/inspect-agent-traces/SKILL.md index a6cbbbe..853f938 100644 --- a/skills/inspect-agent-traces/SKILL.md +++ b/skills/inspect-agent-traces/SKILL.md @@ -5,13 +5,12 @@ description: Inspect real agent workflows with the published Traces CLI and expo # Inspect agent traces -Use the deterministic CLI first. -Keep inspection local and read-only. +Use the deterministic CLI first; keep inspection local and read-only. ## Choose the way in -If the trace is already OTLP (a system that emits `@tangle-network/agent-trace-contract` -spans, or any conforming exporter), read it directly. No adapter is involved. +If the trace is already OTLP (any `@tangle-network/agent-trace-contract` emitter or other +conforming exporter), read it directly. No adapter is involved. ```bash traces validate spans.otlp.jsonl # what can this trace answer? exit 1 on error findings @@ -22,14 +21,13 @@ traces analyze --otlp results/sessions --out .traces/all.md # a directory of e Use `--harness` only for coding agents whose on-disk format we do not control. Those adapters are the legacy edge, not the way to integrate a system you own. -A run directory holds the span export beside raw event, stream and SDK logs that are -also `*.jsonl`. Only the OTLP files are read; the rest are listed with what they hold. -An `otlp/` subdirectory, when present, is read on its own. +In a run directory only the OTLP files are read; other `*.jsonl` logs are listed with +what they hold. An `otlp/` subdirectory, when present, is read on its own. -Any section headed `inputs incomplete`, or carrying an `Inputs incomplete` line above -its table, is computed from a field the trace does not carry everywhere. Report those -numbers as uncaptured, never as zero spend. The `trace conformance` section at the top -names every such capability once; the markers repeat it where the number actually is. +A section headed `inputs incomplete`, or carrying an `Inputs incomplete` line above its +table, is computed from a field the trace does not carry everywhere. Report those numbers +as uncaptured, never as zero spend. The `trace conformance` section names each such +capability once; the markers repeat it at the number. For a loop trace, read `round-over-round convergence` (did round N+1 improve on N) and `steering chain` (which verdict caused which retry) before drawing any conclusion about @@ -48,7 +46,7 @@ traces analyze --harness codex --current --latest-turn --workflow \ - Use `--session ` to pin a listed session. - Use `--latest-turn` for the current task in a resumed Codex or Claude Code session. - Use `--workflow` to include workers linked by stable parent and child IDs. -- Use `--max-workflow-sessions ` only when the default 100-file bound is insufficient. +- Use `--max-workflow-sessions ` only when the default 100-file bound is too small. - For Claude Code, use `--harness claude-code --session --latest-turn`; nested subagents are included. Never join agents by display name or timestamp when Traces reports missing or conflicting IDs. @@ -70,7 +68,7 @@ traces improve --harness codex --current --latest-turn --workflow \ ``` `improve` writes findings, evidence, a report, and spans. -It does not edit an agent, repository, memory store, or knowledge base. +It edits no agent, repository, memory store, or knowledge base. Write one session's durable evidence directory for a later reader: @@ -80,12 +78,11 @@ traces bundle --harness claude-code --session --out .traces/bundle `bundle` copies the transcript, the derived report and spans, the `.evolve` ledger rows inside the session window, and a `manifest.json` with a SHA-256 per file. It spends no model call. -A missing transcript stops the assembly. -An absent optional input is recorded in `manifest.absent` with the probed path. +A missing transcript stops the assembly; an absent optional input is recorded in `manifest.absent` with the probed path. ## Pick the view for the reader -That bundle is the FULL view (`manifest.view: "full"`), and it holds the whole session transcript. +That bundle is the FULL view (`manifest.view: "full"`) and holds the whole session transcript. Never give it to a writer that must not see an earlier conclusion: the transcript holds the text of every file the session wrote, so a check for the earlier report FILE passes while its CONTENT is still readable. Project the writer's copy instead: @@ -94,17 +91,22 @@ Project the writer's copy instead: traces bundle-view .traces/bundle --view evidence-only --out .traces/writer ``` -That view carries `derived/session-index.json`, `derived/evidence.jsonl`, and the structured `ledger/` records. -It excludes the transcripts, the report, the spans, and every prose ledger file by name, with rules and hashes in `manifest.excluded`. -Before writing, it compares each carried file against each excluded file for shared 8-word runs of prose, and drops any file that repeats one. +It carries `derived/session-index.json`, `derived/evidence.jsonl`, and the structured `ledger/` records. +It excludes the transcripts, report, spans, and every prose ledger file by name, with the rules and hashes in `manifest.excluded`. +It also drops any carried file that repeats an 8-word run of prose from an excluded one. `manifest.view` names which copy you hold. +## Ask your own question + +`traces ask --last 1 --question "" --budget 2 --dir .traces/ask` keeps the engine's +prose answer, resolves every `trace://` citation in it, and spends model calls to do it. + ## Report - State the source (`--otlp ` or the harness), selected task boundary, session and span counts, and integrity warnings. - State which capabilities the trace could not support, and name the analyses that reported nothing because of it. - Cite each finding with its exact `trace://` reference. -- Mark missing outcome, cost, token, skill, or relationship data as unknown. +- Mark missing outcome, cost, token, skill, or relationship data unknown. - Do not infer task success from a completion message. - Do not infer skill use from reading a `SKILL.md`. - Do not upload traces unless explicitly requested. diff --git a/src/ask.ts b/src/ask.ts index a0c0cab..66abd47 100644 --- a/src/ask.ts +++ b/src/ask.ts @@ -118,8 +118,10 @@ export interface TraceQuestionsTotals { readonly providerCalls: number /** Total spend with its provenance; `uncaptured` carries a null amount, never 0. */ readonly cost: CostProvenance - /** Wall time of the whole run. */ + /** Wall time of the whole run, from the first span written to the last answer. */ readonly wallTimeMs: number + /** Part of `wallTimeMs` spent writing and indexing the trace file, before any question ran. */ + readonly setupTimeMs: number /** Sum of the questions' own latencies; above `wallTimeMs` when questions overlapped. */ readonly questionTimeMs: number /** Most questions observed running at the same time. */ @@ -132,7 +134,10 @@ export interface TraceQuestionsResult { readonly generatedAt: string readonly harness: string readonly engine: { readonly id: string; readonly version: string; readonly model: string | null } + /** Questions the run was allowed to overlap, as requested. */ readonly concurrency: number + /** Workers the run actually created: `min(concurrency, questions)`. */ + readonly effectiveConcurrency: number /** Shared ceiling across every question; null when uncapped. */ readonly budgetUsd: number | null /** The engine's own ceiling for one question, when it declares one. */ @@ -215,16 +220,29 @@ const QUESTION_ID = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/ const QUESTION_KEYS = new Set(['id', 'question', 'instructions', 'answerSchema']) +/** The first `q` from `index + 1` that no explicit or earlier default ID holds. */ +function defaultQuestionId(index: number, reserved: ReadonlySet, taken: ReadonlySet): string { + let n = index + 1 + while (reserved.has(`q${n}`) || taken.has(`q${n}`)) n += 1 + return `q${n}` +} + /** * Validate questions and assign default IDs. Throws on an empty list, a * duplicate or malformed ID, a question too long for the preview, or an answer * schema outside the supported subset. + * + * Explicit IDs are reserved before any default is assigned, so a file entry + * named `q2` plus a positional question cannot collide and kill the run before + * it starts. A default keeps its own position where it can: it starts at + * `q` and takes the next free number only when that one is spoken for. */ export function normalizeTraceQuestions(questions: readonly TraceQuestion[]): Array { if (questions.length === 0) throw new Error('ask needs at least one question') + const reserved = new Set(questions.flatMap((entry) => (typeof entry.id === 'string' ? [entry.id] : []))) const seen = new Set() return questions.map((entry, index) => { - const id = entry.id ?? `q${index + 1}` + const id = entry.id ?? defaultQuestionId(index, reserved, seen) if (!QUESTION_ID.test(id)) { throw new Error(`question ID "${id}" must match ${QUESTION_ID} (letters, digits, dot, underscore, hyphen)`) } @@ -379,11 +397,14 @@ export function traceCitationsInText(text: string): Array<{ uri: string; traceId return out } -async function verifyCitations( - text: string, - store: TraceAnalysisStore, - signal: AbortSignal | undefined, -): Promise { +/** + * Resolve every citation in an answer against the store. + * + * No signal: the index is built before the first question runs, so this is an + * in-memory lookup, and forwarding an aborted run signal here would make + * `store.hasSpans` throw and discard an answer that was already paid for. + */ +async function verifyCitations(text: string, store: TraceAnalysisStore): Promise { const citations = traceCitationsInText(text) const wanted = new Map>() for (const citation of citations) { @@ -394,7 +415,7 @@ async function verifyCitations( } const found = new Map>() for (const [traceId, spanIds] of wanted) { - const existing = await store.hasSpans({ trace_id: traceId, span_ids: [...spanIds] }, signal ? { signal } : undefined) + const existing = await store.hasSpans({ trace_id: traceId, span_ids: [...spanIds] }) found.set(traceId, new Set(existing)) } return citations.map((citation) => ({ @@ -411,6 +432,22 @@ function isBudgetRefusal(error: unknown): boolean { return /would exceed ceiling|model cost limit reached/.test(message) } +/** + * True when the shared ledger could no longer admit one model call at the time + * the question failed. + * + * Message matching alone is not enough: the refusal happens inside the model + * proxy, behind the bridge, and the bridge's HTTP error handling can replace + * the Node error text with its own. Reconciling against the ledger's settled + * spend names the cause from the accounting rather than from the wording, so + * an exhausted budget reads as `budget-refused` however the failure surfaced. + */ +function ledgerIsExhausted(ledger: CostLedger, budgetUsd: number | undefined, floorUsd: number | undefined): boolean { + if (budgetUsd === undefined || floorUsd === undefined) return false + const settled = ledger.summary({ channel: 'analyst' }).totalCostUsd + return Number.isFinite(settled) && budgetUsd - settled < floorUsd +} + function errorMessage(error: unknown): string { return error instanceof Error ? `${error.constructor.name}: ${error.message}` : String(error) } @@ -436,6 +473,32 @@ function formatUsd(value: number): string { return `$${value < 0.01 && value > 0 ? value.toPrecision(2) : value.toFixed(2)}` } +/** A question the pool never reached, recorded so the run still accounts for it. */ +function unrunAnswer( + question: TraceQuestion & { id: string }, + failure: NonNullable, + model: string | null, +): TraceQuestionAnswer { + const at = new Date().toISOString() + return { + id: question.id, + question: question.question, + status: 'failed', + failure, + answer: null, + citations: [], + findings: [], + rejectedFindings: {}, + model, + modelCalls: null, + toolCalls: null, + usage: null, + startedAt: at, + endedAt: at, + latencyMs: 0, + } +} + function sumOrNull(values: ReadonlyArray): number | null { return values.some((value) => value === null) ? null : values.reduce((sum, value) => sum + (value ?? 0), 0) } @@ -443,8 +506,11 @@ function sumOrNull(values: ReadonlyArray): number | null { /** * Ask every question of the spans, concurrently, under one shared cost * ledger. Never throws for a failed question: each failure is recorded on its - * answer and `ok` turns false. Throws before any model call for invalid - * questions, options, or a budget below one call's reservation. + * answer and `ok` turns false. An aborted run is one of those failures, not an + * exception: answers already bought are kept and the questions the run never + * reached are recorded as `aborted`, so the caller can still write both + * artifacts. Throws before any model call for invalid questions, options, or a + * budget below one call's reservation. */ export async function runTraceQuestions(opts: TraceQuestionsOptions): Promise { if (opts.spans.length === 0) throw new Error('runTraceQuestions: no spans to ask about') @@ -475,6 +541,10 @@ export async function runTraceQuestions(opts: TraceQuestionsOptions): Promise 0 && !failure) { + failure = { kind: 'invalid-answer', message: problems.slice(0, 5).join('; ') } + } } - if (problems.length > 0 && !failure) { - failure = { kind: 'invalid-answer', message: problems.slice(0, 5).join('; ') } + const unresolved = citations.filter((citation) => !citation.resolved) + if (unresolved.length > 0 && !failure) { + failure = { + kind: 'unresolved-citations', + message: `${unresolved.length} cited span(s) do not exist: ${unresolved.slice(0, 3).map((c) => c.uri).join(', ')}`, + } } + } else if (completed && !failure) { + failure = { kind: 'no-answer', message: 'the engine returned an empty answer' } } - const unresolved = citations.filter((citation) => !citation.resolved) - if (unresolved.length > 0 && !failure) { + } catch (error) { + if (!failure) { failure = { - kind: 'unresolved-citations', - message: `${unresolved.length} cited span(s) do not exist: ${unresolved.slice(0, 3).map((c) => c.uri).join(', ')}`, + kind: opts.signal?.aborted ? 'aborted' : 'error', + message: `checking the answer failed: ${errorMessage(error)}`, } } - } else if (completed && !failure) { - failure = { kind: 'no-answer', message: 'the engine returned an empty answer' } } const endedAt = new Date() return { @@ -598,15 +684,32 @@ export async function runTraceQuestions(opts: TraceQuestionsOptions): Promise(questions.length) let next = 0 - const runStarted = performance.now() - await Promise.all(Array.from({ length: effectiveConcurrency }, async () => { - while (next < questions.length) { - const index = next - next += 1 - answers[index] = await askOne(questions[index]!) - } - })) + const setupTimeMs = Math.round(performance.now() - runStarted) + let poolError: unknown + try { + await Promise.all(Array.from({ length: effectiveConcurrency }, async () => { + while (next < questions.length) { + const index = next + next += 1 + answers[index] = await askOne(questions[index]!) + } + })) + } catch (error) { + poolError = error + } const wallTimeMs = Math.round(performance.now() - runStarted) + // `askOne` records every failure on its own answer, so the pool is not + // expected to reject. If it ever does, the answers already bought are still + // returned and written rather than lost with the exception: the caller sees + // the cause in the warnings and in each question the pool never reached. + if (poolError !== undefined) { + const message = errorMessage(poolError) + warnings.push(`the question pool stopped early: ${message}`) + const kind: TraceQuestionFailureKind = opts.signal?.aborted ? 'aborted' : 'error' + for (const [index, question] of questions.entries()) { + answers[index] ??= unrunAnswer(question, { kind, message }, opts.engine.model ?? null) + } + } const summary = ledger.summary({ channel: 'analyst' }) const answered = answers.filter((answer) => answer.status === 'answered').length @@ -619,6 +722,7 @@ export async function runTraceQuestions(opts: TraceQuestionsOptions): Promise sum + answer.latencyMs, 0), peakConcurrency, } @@ -629,6 +733,7 @@ export async function runTraceQuestions(opts: TraceQuestionsOptions): Promise: ` section sits at. */ +const ANSWER_SECTION_LEVEL = 2 + +/** + * Demote the answer's own Markdown headings below the question's section. + * + * An answer is model prose that may itself be Markdown. Embedded verbatim, its + * `# Heading` would close the question's section and reflow the rest of the + * report under the model's outline. Fenced blocks are left alone: a `#` there + * is content, not a heading. + */ +function nestAnswerHeadings(text: string): string { + let fence: { marker: string; length: number } | null = null + return text.split('\n').map((line) => { + const fenced = /^ {0,3}(`{3,}|~{3,})/.exec(line) + if (fenced) { + const marker = fenced[1]![0]! + const length = fenced[1]!.length + if (fence === null) fence = { marker, length } + else if (fence.marker === marker && length >= fence.length) fence = null + return line + } + if (fence !== null) return line + const heading = /^(#{1,6})(?=\s)/.exec(line) + if (!heading) return line + const level = Math.min(6, Math.max(ANSWER_SECTION_LEVEL + 1, heading[1]!.length)) + return `${'#'.repeat(level)}${line.slice(heading[1]!.length)}` + }).join('\n') +} + /** Readable Markdown for a result; the JSON result carries every field. */ export function renderTraceQuestionsReport(result: Omit): string { const { totals } = result const lines = ['# traces ask', ''] + // The requested limit and the workers actually created differ whenever there + // are fewer questions than the limit, and only the second one bounds overlap. + const effective = result.effectiveConcurrency === result.concurrency ? '' : ` (${result.effectiveConcurrency} effective)` lines.push( `${totals.questions} question(s) over ${result.traces.length} trace(s) (${result.spanCount} spans, ${result.harness}). ` + `Engine \`${result.engine.id}\`${result.engine.model ? `, model \`${result.engine.model}\`` : ''}; ` + - `concurrency ${result.concurrency}; budget ${result.budgetUsd === null ? 'uncapped' : `${formatUsd(result.budgetUsd)} shared`}` + + `concurrency ${result.concurrency}${effective}; ` + + `budget ${result.budgetUsd === null ? 'uncapped' : `${formatUsd(result.budgetUsd)} shared`}` + `${result.questionBudgetUsd === null ? '' : `, ${formatUsd(result.questionBudgetUsd)} per question`}.`, ) lines.push('') lines.push( `**${totals.answered} answered, ${totals.failed} failed.** Wall time ${seconds(totals.wallTimeMs)} ` + + `(${seconds(totals.setupTimeMs)} of it writing and indexing the trace file) ` + `against ${seconds(totals.questionTimeMs)} of question time (peak ${totals.peakConcurrency} at once). ` + `Cost ${costText(totals.cost)} over ${totals.providerCalls} provider call(s); ` + `${countText(totals.modelCalls)} model call(s), ${countText(totals.toolCalls)} tool call(s).`, @@ -703,7 +843,7 @@ export function renderTraceQuestionsReport(result: Omit { * ledger, so `--budget` bounds the whole run and `--question-budget` bounds * each question. The answers, their citation checks, and per-question cost and * time are written before the exit code is decided, so a failed question never - * costs the others' answers. + * costs the others' answers. Ctrl-C is one of those failures: `runTraceQuestions` + * records it on the questions the run never reached and still returns, so the + * artifacts below are written for the answers already paid for. */ async function cmdAsk(args: Args): Promise { if (args.out) throw new Error('ask writes a directory of artifacts; pass --dir instead of --out') diff --git a/src/report.ts b/src/report.ts index 94425ff..7b56a76 100644 --- a/src/report.ts +++ b/src/report.ts @@ -193,6 +193,9 @@ function tableCell(value: string): string { const ANALYST_DETAIL_MAX_CHARS = 240 +/** Room the condensed engine error keeps when a rejection summary shares the cell. */ +const ANALYST_DETAIL_MIN_ERROR_CHARS = 120 + /** * Failure-reason marker printed by newer (currently unreleased) agent-eval * bridges. The pinned agent-eval-rpc release never emits it, so the head+tail @@ -234,13 +237,24 @@ export function analystRunDetail(summary: AnalystRunSummary, rejections?: Findin : summary.status === 'skipped' && summary.reason ? summary.reason : '' - const detail = condenseAnalystError(raw, ANALYST_DETAIL_MAX_CHARS) - // The rejection text is short and bounded by the gate's reason vocabulary, - // so it is appended after the condensed error rather than competing for it. - const cell = tableCell([detail, formatFindingRejections(rejections)].filter(Boolean).join('; ')) + // One budget for the whole cell. The gate's reason text comes from the + // engine and is not bounded at its source, so it is capped first and the + // condensed error takes what is left; the cell keeps the length + // ANALYST_DETAIL_MAX_CHARS names whether or not both parts are present. + const rejectionRoom = raw ? ANALYST_DETAIL_MAX_CHARS - ANALYST_DETAIL_MIN_ERROR_CHARS - 2 : ANALYST_DETAIL_MAX_CHARS + const rejected = truncateChars(formatFindingRejections(rejections), rejectionRoom) + const detail = condenseAnalystError(raw, ANALYST_DETAIL_MAX_CHARS - (rejected ? rejected.length + 2 : 0)) + const cell = tableCell([detail, rejected].filter(Boolean).join('; ')) return cell === '' ? '—' : cell } +/** Code-point-safe truncation, so a boundary never splits a surrogate pair. */ +function truncateChars(text: string, maxChars: number): string { + const chars = [...text] + if (chars.length <= maxChars) return text + return `${chars.slice(0, Math.max(0, maxChars - 1)).join('')}…` +} + function count(value: number): string { return Math.round(value).toLocaleString('en-US') } diff --git a/tests/ask.test.ts b/tests/ask.test.ts index 8d23d30..8eb2ea9 100644 --- a/tests/ask.test.ts +++ b/tests/ask.test.ts @@ -229,7 +229,10 @@ describe('runTraceQuestions', () => { expect(result.totals.questionTimeMs).toBeGreaterThanOrEqual(1_200) expect(result.totals.wallTimeMs).toBeLessThan(result.totals.questionTimeMs / 2) expect(result.totals.peakConcurrency).toBe(6) - expect(result.report).toMatch(/Wall time \d+\.\d s against \d+\.\d s of question time \(peak 6 at once\)/) + expect(result.report).toMatch( + /Wall time \d+\.\d s \(\d+\.\d s of it writing and indexing the trace file\) against \d+\.\d s of question time \(peak 6 at once\)/, + ) + expect(result.totals.setupTimeMs).toBeLessThanOrEqual(result.totals.wallTimeMs) }) it('reports questions the shared ledger refused while the others keep their answers', async () => { @@ -326,6 +329,124 @@ describe('runTraceQuestions', () => { .toEqual(['answer q1', 'answer q2', 'answer q4', 'answer q5']) }) + it('keeps an answer bought before an abort, and writes both artifacts', async () => { + const controller = new AbortController() + let aborted = false + const { engine } = scriptedEngine(async () => { + // Ctrl-C lands once the first answer is back, before the next question starts. + if (!aborted) { + aborted = true + queueMicrotask(() => controller.abort(new Error('ctrl-c'))) + } + return { answer: `The command ran (trace://${TRACE}/span/tool-1).` } + }) + const dir = await mkdtemp(join(tmpdir(), 'traces-ask-abort-')) + const result = await runTraceQuestions({ + questions: [{ question: 'Which commands ran?' }, { question: 'Which failed?' }, { question: 'What was last?' }], + spans: fixtureSpans(), + engine, + concurrency: 1, + otlpOutPath: join(dir, 'traces.otlp.jsonl'), + signal: controller.signal, + }) + + // The citation check must not carry the run signal: it is an in-memory + // lookup, and throwing there would discard an answer already paid for. + expect(result.questions.map((answer) => [answer.id, answer.status, answer.failure?.kind ?? null])).toEqual([ + ['q1', 'answered', null], + ['q2', 'failed', 'aborted'], + ['q3', 'failed', 'aborted'], + ]) + expect(result.questions[0]!.citations).toEqual([ + { uri: `trace://${TRACE}/span/tool-1`, traceId: TRACE, spanId: 'tool-1', resolved: true }, + ]) + expect(result.ok).toBe(false) + + const artifacts = await writeTraceQuestionsArtifacts(result, dir) + const saved = JSON.parse(await readFile(artifacts.result, 'utf8')) as typeof result + expect(saved.questions.map((answer) => answer.status)).toEqual(['answered', 'failed', 'failed']) + expect(await readFile(artifacts.report, 'utf8')).toContain(result.questions[0]!.answer!) + }) + + it('names an exhausted shared budget from the ledger when the failure text does not', async () => { + const { engine } = scriptedEngine(async (request) => { + if (questionId(request) === 'q1') { + const paid = await request.costLedger.runPaidCall({ + channel: 'analyst', + phase: request.costPhase, + actor: request.analystId, + ...(request.costTags ? { tags: request.costTags } : {}), + maximumCharge: { externallyEnforcedMaximumUsd: 0.7 }, + execute: async () => 'ok', + receipt: () => ({ model: 'test-model', inputTokens: 100, outputTokens: 50, actualCostUsd: 0.7 }), + }) + if (!paid.succeeded) throw paid.error + return { answer: 'answer q1' } + } + // The bridge's own HTTP error handling replaces the Node error text, so + // nothing in the message names the ceiling that actually stopped the call. + throw new Error('DSPY-BRIDGE-FAILURE: HTTPError: 500 Server Error for url: http://127.0.0.1/call') + }, { + pricing: { inputUsdPerMillion: 1.25, outputUsdPerMillion: 10 }, + max_output_tokens: 8_192, + max_reasoning_tokens: 32_768, + }) + const result = await runTraceQuestions({ + questions: [{ question: 'Which commands ran?' }, { question: 'Which failed?' }], + spans: fixtureSpans(), + engine, + concurrency: 1, + budgetUsd: 1, + }) + + // $0.70 settled leaves $0.30, below the $0.41 one call reserves. + expect(result.questions[0]!.status).toBe('answered') + expect(result.questions[1]!.failure).toEqual({ + kind: 'budget-refused', + message: expect.stringContaining('500 Server Error'), + }) + expect(result.report).toContain('failed: budget-refused') + }) + + it('nests an answer\'s own headings under its question section, leaving fenced text alone', async () => { + const answer = [ + '# traces ask', + '', + '| command | exit |', + '| --- | --- |', + '| pnpm test | 0 |', + '', + '```sh', + '# not a heading', + '```', + ].join('\n') + const { engine } = scriptedEngine(async () => ({ answer })) + const result = await runTraceQuestions({ + questions: [{ id: 'commands', question: 'Which commands ran?' }], + spans: fixtureSpans(), + engine, + }) + // The model's own `#` heading would otherwise close the question's section + // and pull the rest of the report under the answer's outline. + expect(result.report).toContain('## commands: Which commands ran?') + expect(result.report).toContain('### traces ask') + expect(result.report).not.toContain('\n# traces ask\n\n| command | exit |') + expect(result.report).toContain('```sh\n# not a heading\n```') + }) + + it('reports the effective concurrency next to the requested one', async () => { + const { engine } = scriptedEngine(async () => ({ answer: 'one worker was enough' })) + const result = await runTraceQuestions({ + questions: [{ question: 'Which commands ran?' }], + spans: fixtureSpans(), + engine, + concurrency: 8, + }) + expect(result.concurrency).toBe(8) + expect(result.effectiveConcurrency).toBe(1) + expect(result.report).toContain('concurrency 8 (1 effective)') + }) + it('fails an answer that cites a span the trace does not hold', async () => { const { engine } = scriptedEngine(async () => ({ answer: `Tests ran at trace://${TRACE}/span/tool-1 and trace://${TRACE}/span/invented-span.`, @@ -465,6 +586,11 @@ describe('question input', () => { it('assigns default IDs and rejects duplicates', () => { expect(normalizeTraceQuestions([{ question: ' a? ' }, { question: 'b?' }]).map((entry) => [entry.id, entry.question])) .toEqual([['q1', 'a?'], ['q2', 'b?']]) + // A file entry named q2 and a positional question must not both claim q2. + expect(normalizeTraceQuestions([{ id: 'q2', question: 'a?' }, { question: 'b?' }]).map((entry) => entry.id)) + .toEqual(['q2', 'q3']) + expect(normalizeTraceQuestions([{ question: 'a?' }, { id: 'q1', question: 'b?' }]).map((entry) => entry.id)) + .toEqual(['q2', 'q1']) expect(() => normalizeTraceQuestions([{ id: 'x', question: 'a?' }, { id: 'x', question: 'b?' }])) .toThrow('duplicate question ID "x"') expect(() => normalizeTraceQuestions([])).toThrow('at least one question') diff --git a/tests/report.test.ts b/tests/report.test.ts index eb57036..b712d68 100644 --- a/tests/report.test.ts +++ b/tests/report.test.ts @@ -655,6 +655,22 @@ describe('analystRunDetail', () => { expect(detail).toContain(' … ') }) + it('keeps the cell inside its length budget when a rejection summary shares it', () => { + const detail = analystRunDetail({ + analyst_id: 'failure-mode', + status: 'failed', + findings_count: 0, + latency_ms: 1, + usage, + error: { class: 'Error', message: 'DSPy RLM trace analysis exited 1. stderr='.repeat(20) }, + }, { [`the cited span holds no text matching the excerpt ${'x'.repeat(400)}`]: 3 }) + // Both parts survive, and the cell keeps the same bound a lone condensed + // error keeps: ANALYST_DETAIL_MAX_CHARS plus the ' … ' join. + expect(detail).toContain('DSPy RLM trace analysis exited 1.') + expect(detail).toContain('3 finding(s) rejected:') + expect([...detail].length).toBeLessThanOrEqual(243) + }) + it('renders a whitespace-only error as the empty-cell dash', () => { expect(analystRunDetail({ analyst_id: 'failure-mode', From 541e2eb4404ab2b531d649ffd4defc2282ca6fe5 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Thu, 10 Sep 2026 02:06:02 -0700 Subject: [PATCH 2/2] fix(ask): resolve a citation the model wrapped in Markdown emphasis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `traceCitationsInText` trimmed only `.:!?` from a matched URI, and the span-id character class admits `*`, `_` and `~`. An answer that cited a real span as `**trace:///span/**` therefore carried the closing delimiters into the span ID, resolved against nothing, and failed the question with `unresolved-citations` — so `traces ask` exited 1 on a correct answer, and the command's headline guarantee returned a wrong verdict. The trailing strip now covers the Markdown emphasis run. Also from the same review, each with its own test: - `findingRejectionDetail` reads the cause from `reason` for any rejection kind and from `issues` for a schema failure, and adds only what the message does not already say, so the CLI line no longer degenerates to "finding rejected: schema failure — schema failure". - A failure whose own text names its cause is no longer relabelled `budget-refused` by the ledger reconciliation: a bridge mismatch stays `error` (keeping the CLI's reinstall hint, which now fires on the message whatever the kind) and an empty bridge answer maps to the `no-answer` kind, which was unreachable before. - `cmdAsk` builds the engine before the adapter pass, so a missing TANGLE_API_KEY is reported before a large session is parsed. - A multi-line question is folded onto one line in the report heading; `answers.json` keeps it verbatim. - The worker pool awaits every worker with `allSettled`, so a rejection cannot hand back an answers array the remaining workers still write to. - `assertAnswerSchema` rejects `required`, `properties` or `additionalProperties` without `"type": "object"`, and `items` without `"type": "array"`: those constraints are skipped for an answer of any other shape, so `{ required: ['a'] }` accepted the answer `5`. Co-Authored-By: Claude Opus 5 (1M context) --- docs/trace-analysts.md | 7 ++- src/answer-schema.ts | 20 ++++++- src/ask.ts | 80 ++++++++++++++++++------- src/cli.ts | 16 +++-- src/finding-rejections.ts | 26 +++++++- tests/ask.test.ts | 100 +++++++++++++++++++++++++++++++ tests/finding-rejections.test.ts | 17 +++++- 7 files changed, 232 insertions(+), 34 deletions(-) diff --git a/docs/trace-analysts.md b/docs/trace-analysts.md index 810b682..0a0e9a7 100644 --- a/docs/trace-analysts.md +++ b/docs/trace-analysts.md @@ -109,12 +109,14 @@ Read many questions from a file: An `answerSchema` makes the answer one JSON value a scorer can compare field by field. The supported keywords are `type`, `properties`, `required`, `additionalProperties`, `items`, `enum`, `const`, `title`, and `description`. +A keyword that constrains one JSON type must declare it: `required`, `properties`, and `additionalProperties` need `"type": "object"`, and `items` needs `"type": "array"`, or the constraint would be skipped for an answer of another shape. Any other keyword is rejected when the run starts. This package carries no JSON Schema library, and a constraint that is quietly ignored would let a wrong answer pass as checked. ### What the run guarantees - Every `trace:///span/` URI in an answer is resolved against the store. An unresolvable citation fails that question. + A citation the model wrapped in Markdown emphasis (`**...**`, `_..._`, `~~...~~`) or ended a sentence with resolves like a bare one: the delimiters are prose, not part of the span ID. - Findings the answer submits still pass the same evidence gate as the built-in kinds. Refused findings are counted by reason in both artifacts. - A failed question never stops the others. Its failure is recorded on its own answer, and the remaining answers are written. - Ctrl-C keeps what the run already bought. The signal reaches the engine, not the checks that follow it: an answer that came back is kept with its citations resolved, and each question the run never reached is recorded as `aborted`. @@ -133,7 +135,8 @@ Two consequences follow. A question the ledger refuses is reported as `budget-refused`, and the answers already produced are kept. That kind is decided from the accounting, not only from the error text. The refusal happens inside the model proxy, behind the DSPy bridge, whose HTTP error handling can replace the Node error with its own message. -So a failed question is reported as `budget-refused` whenever the shared ledger's settled spend left less than one model call's reservation at the time it failed, whatever the message says. +So a failed question is reported as `budget-refused` whenever the shared ledger's settled spend left less than one model call's reservation at the time it failed and nothing else in the failure names its cause. +A failure whose own text names its cause keeps that cause: a bridge version mismatch stays `error`, so the reinstall hint still prints, and an empty answer from the bridge stays `no-answer`. The message itself is kept verbatim on the answer. ### SDK @@ -165,7 +168,7 @@ Without the reason, a report showing "0 findings" reads as "the model found noth `analyze`, `investigate`, `improve`, and `ask` now carry those refusals: -- the CLI log prints the reason and the offending URI on each `finding rejected` line; +- the CLI log adds whatever the event carries beyond its own text to each `finding rejected` line: the cause (`reason` for the gate's rejections and the bridge-row rejection, `issues` for a schema failure), the offending URI, the citation counts, and the subject; - the analyst table's Detail cell names the reasons and their counts; - `result.findingRejections` (investigation and improvement) and `answers.json` (`ask`) hold the counts per analyst and reason. diff --git a/src/answer-schema.ts b/src/answer-schema.ts index 869e727..4dae947 100644 --- a/src/answer-schema.ts +++ b/src/answer-schema.ts @@ -9,7 +9,9 @@ * * Supported keywords: `type`, `properties`, `required`, `additionalProperties` * (boolean), `items` (one schema), `enum`, `const`, and the annotations - * `title` and `description`. + * `title` and `description`. The four keywords that constrain one JSON type + * must declare that type: `required` without `"type": "object"`, or `items` + * without `"type": "array"`, checks nothing against an answer of another shape. */ export type AnswerSchema = Readonly> @@ -51,6 +53,22 @@ export function assertAnswerSchema(schema: unknown, path = 'answerSchema', depth throw new TypeError(`${path}.type must be one of ${[...JSON_TYPES].join(', ')}, or an array of them`) } } + // `properties`, `required`, `additionalProperties` and `items` constrain one + // JSON type each and are skipped for every other type. A schema that carries + // one without declaring that type checks nothing at all against an answer of + // the wrong shape — `{ required: ['a'] }` would accept the answer `5` — so it + // is rejected here rather than passing a wrong answer as checked. + const declared = schema.type === undefined + ? undefined + : (Array.isArray(schema.type) ? schema.type : [schema.type]) as string[] + for (const keyword of ['properties', 'required', 'additionalProperties'] as const) { + if (schema[keyword] !== undefined && !declared?.includes('object')) { + throw new TypeError(`${path}: "${keyword}" is checked only for an object; declare "type": "object" alongside it`) + } + } + if (schema.items !== undefined && !declared?.includes('array')) { + throw new TypeError(`${path}: "items" is checked only for an array; declare "type": "array" alongside it`) + } if (schema.properties !== undefined) { if (!isRecord(schema.properties)) throw new TypeError(`${path}.properties must be an object`) for (const [name, child] of Object.entries(schema.properties)) { diff --git a/src/ask.ts b/src/ask.ts index 66abd47..1ff6cdc 100644 --- a/src/ask.ts +++ b/src/ask.ts @@ -34,6 +34,7 @@ import { formatFindingRejections, totalFindingRejections, } from './finding-rejections.js' +import { isBridgeMismatchError } from './improvement.js' import type { OtlpSpan } from './otlp.js' /** One question to ask of the selected traces. */ @@ -206,6 +207,8 @@ const ASK_RULES = [ 'TRACES ASK RULES', '1. Answer QUESTION only from trace tool results retrieved in this run.', '2. PREPARED CONTEXT: near the end of analyst_instructions holds JSON listing every trace. Parse it; copy IDs from it or from tool output.', + // The rules must fit the DSPy preview head, so this one does not also legislate + // formatting: `traceCitationsInText` reads a citation the model emphasised. '3. Cite each fact as trace:///span/. Every cited span must exist.', '4. Quote excerpts from viewSpans output, never from searchTrace hits.', '5. If the trace does not record a fact, say "not in trace".', @@ -371,13 +374,24 @@ function preparedContext(traces: readonly TraceQuestionTrace[]): string { const TRACE_URI = /trace:\/\/[^\s/"'`<>()[\]{}]+\/span\/[^\s/"'`<>()[\]{},;]+/g +/** + * Trailing characters that are prose around a citation, not part of the span ID. + * + * Sentence punctuation is the obvious case. The Markdown emphasis run matters + * just as much: a model that writes `**trace://t/span/s**` or `_trace://t/span/s_` + * has cited a real span, and keeping its closing delimiters in the ID makes a + * correct answer fail with `unresolved-citations` and drives `ask` to exit 1. + * Adapter-assigned span and trace IDs do not end in these characters, so + * trimming them cannot hide a citation that would otherwise resolve. + */ +const TRAILING_PROSE = /[.,:;!?*_~]+$/ + /** Every distinct `trace:///span/` URI in the text, in order. */ export function traceCitationsInText(text: string): Array<{ uri: string; traceId: string | null; spanId: string | null }> { const seen = new Set() const out: Array<{ uri: string; traceId: string | null; spanId: string | null }> = [] for (const match of text.matchAll(TRACE_URI)) { - // Sentence punctuation after a URI is prose, not part of the ID. - const uri = match[0].replace(/[.:!?]+$/, '') + const uri = match[0].replace(TRAILING_PROSE, '') if (seen.has(uri)) continue seen.add(uri) const parts = /^trace:\/\/([^/]+)\/span\/([^/]+)$/.exec(uri) @@ -448,6 +462,29 @@ function ledgerIsExhausted(ledger: CostLedger, budgetUsd: number | undefined, fl return Number.isFinite(settled) && budgetUsd - settled < floorUsd } +/** agent-eval's own text for a run that finished with an empty answer field. */ +const ENGINE_NO_ANSWER = /returned no answer/ + +/** + * Name the cause of a failed question. + * + * Order matters. A failure whose own text names its cause keeps that cause: + * the ledger reconciliation is a fallback for a refusal the bridge hid, and + * relabelling a bridge-version mismatch as `budget-refused` because the budget + * happened to be nearly spent would also suppress the CLI's reinstall hint. + */ +function questionFailureKind( + error: unknown, + state: { aborted: boolean; ledgerExhausted: boolean }, +): TraceQuestionFailureKind { + if (state.aborted) return 'aborted' + if (isBudgetRefusal(error)) return 'budget-refused' + const message = errorMessage(error) + if (isBridgeMismatchError(message)) return 'error' + if (ENGINE_NO_ANSWER.test(message)) return 'no-answer' + return state.ledgerExhausted ? 'budget-refused' : 'error' +} + function errorMessage(error: unknown): string { return error instanceof Error ? `${error.constructor.name}: ${error.message}` : String(error) } @@ -606,11 +643,10 @@ export async function runTraceQuestions(opts: TraceQuestionsOptions): Promise(questions.length) let next = 0 const setupTimeMs = Math.round(performance.now() - runStarted) - let poolError: unknown - try { - await Promise.all(Array.from({ length: effectiveConcurrency }, async () => { - while (next < questions.length) { - const index = next - next += 1 - answers[index] = await askOne(questions[index]!) - } - })) - } catch (error) { - poolError = error - } + // allSettled, not Promise.all: a rejection must not hand the answers array + // back to the caller while the other workers are still writing into it. + const settled = await Promise.allSettled(Array.from({ length: effectiveConcurrency }, async () => { + while (next < questions.length) { + const index = next + next += 1 + answers[index] = await askOne(questions[index]!) + } + })) + const failedWorker = settled.find((worker) => worker.status === 'rejected') const wallTimeMs = Math.round(performance.now() - runStarted) // `askOne` records every failure on its own answer, so the pool is not // expected to reject. If it ever does, the answers already bought are still // returned and written rather than lost with the exception: the caller sees // the cause in the warnings and in each question the pool never reached. - if (poolError !== undefined) { - const message = errorMessage(poolError) + if (failedWorker) { + const message = errorMessage(failedWorker.reason) warnings.push(`the question pool stopped early: ${message}`) const kind: TraceQuestionFailureKind = opts.signal?.aborted ? 'aborted' : 'error' for (const [index, question] of questions.entries()) { @@ -836,7 +870,9 @@ export function renderTraceQuestionsReport(result: Omit): void { // A gate rejection names its cause only in the fields; without them the // line says a finding was dropped but not why, which nobody can act on. const rejection = findingRejectionDetail(msg, fields) - if (rejection) { - process.stderr.write(`${msg} — ${rejection}\n`) + if (rejection !== undefined) { + process.stderr.write(rejection ? `${msg} — ${rejection}\n` : `${msg}\n`) return } const error = typeof fields?.error === 'string' && fields.error ? fields.error : undefined @@ -1271,14 +1271,16 @@ async function cmdAsk(args: Args): Promise { if (args.budget !== undefined && (!Number.isFinite(args.budget) || args.budget <= 0)) { throw new Error('--budget must be a positive number of USD') } - const collected = await collectSpans(args) - if (collected.spans.length === 0) throw new Error('no spans found for the given selection') - warnIncompleteWorkflow(collected.workflow) + // Before the adapter pass: a missing API key must not cost an operator the + // wait for a large session to be parsed before it is reported. const engine = analysisEngineFromEnv({ model: analystModelFor(args), maxCostUsd: args.questionBudget ?? Math.min(args.budget ?? Infinity, DEFAULT_QUESTION_MAX_COST_USD), log: analystLog, }) + const collected = await collectSpans(args) + if (collected.spans.length === 0) throw new Error('no spans found for the given selection') + warnIncompleteWorkflow(collected.workflow) const directory = resolve(args.dir ?? await mkdtemp(join(tmpdir(), 'traces-ask-'))) await mkdir(directory, { recursive: true }) const controller = new AbortController() @@ -1314,7 +1316,9 @@ async function cmdAsk(args: Args): Promise { `${failed.length} of ${result.questions.length} question(s) failed; the answers file holds every result.`, ...failed.map((answer) => ` ${answer.id}: ${answer.failure?.kind}: ${answer.failure?.message.slice(0, 300)}`), ] - if (failed.some((answer) => answer.failure?.kind === 'error' && isBridgeMismatchError(answer.failure.message))) { + // Any kind, not only `error`: the hint is decided by the message, and a + // mismatch that also exhausted the budget still needs the reinstall line. + if (failed.some((answer) => isBridgeMismatchError(answer.failure?.message ?? ''))) { lines.push( 'hint: the DSPy bridge protocol is version-locked — the TRACES_PYTHON interpreter needs ' + `agent-eval-rpc[dspy]==${requiredBridgeVersion()} (matching this package's @tangle-network/agent-eval).`, diff --git a/src/finding-rejections.ts b/src/finding-rejections.ts index 60d89bb..e5b44b8 100644 --- a/src/finding-rejections.ts +++ b/src/finding-rejections.ts @@ -37,14 +37,36 @@ export function findingRejection( return match[1] ? { analystId: match[1], reason } : { reason } } -/** One-line detail for a rejection log event, or undefined for any other event. */ +/** + * The cause an event carries in its fields: `reason` for the gate's own + * rejections and the dspy engine's bridge-row rejection, `issues` for a schema + * failure. Undefined when the event names no cause beyond its kind. + */ +function rejectionCause(fields?: Readonly>): string | undefined { + if (typeof fields?.reason === 'string' && fields.reason) return fields.reason + const issues = fields?.issues + if (typeof issues === 'string' && issues) return issues + if (Array.isArray(issues) && issues.length > 0) return issues.map((issue) => String(issue)).join('; ') + return undefined +} + +/** + * What a rejection log event carries beyond its own message: the cause, the + * offending URI, the citation counts, the subject. Empty when the message + * already says everything known — the kind is in the message, so repeating it + * would print "finding rejected: schema failure — schema failure". Undefined + * for an event that is not a rejection at all. + */ export function findingRejectionDetail( message: string, fields?: Readonly>, ): string | undefined { const rejection = findingRejection(message, fields) if (!rejection) return undefined - const parts = [rejection.reason] + // The cause when the fields name one, the kind otherwise — and only when the + // message does not already end with it. + const lead = rejectionCause(fields) ?? rejection.reason + const parts = message.endsWith(lead) ? [] : [lead] if (typeof fields?.uri === 'string' && fields.uri) parts.push(`uri ${fields.uri}`) if (typeof fields?.required === 'number' && typeof fields?.distinct === 'number') { parts.push(`${fields.distinct} of ${fields.required} required distinct citation(s)`) diff --git a/tests/ask.test.ts b/tests/ask.test.ts index 8eb2ea9..dc072e8 100644 --- a/tests/ask.test.ts +++ b/tests/ask.test.ts @@ -408,6 +408,44 @@ describe('runTraceQuestions', () => { expect(result.report).toContain('failed: budget-refused') }) + it('keeps a failure that names its own cause out of the budget reconciliation', async () => { + const bridge = 'DSPY-BRIDGE-FAILURE: RuntimeError: could not start the bridge' + const { engine } = scriptedEngine(async (request) => { + if (questionId(request) === 'q1') { + const paid = await request.costLedger.runPaidCall({ + channel: 'analyst', + phase: request.costPhase, + actor: request.analystId, + ...(request.costTags ? { tags: request.costTags } : {}), + maximumCharge: { externallyEnforcedMaximumUsd: 0.7 }, + execute: async () => 'ok', + receipt: () => ({ model: 'test-model', inputTokens: 100, outputTokens: 50, actualCostUsd: 0.7 }), + }) + if (!paid.succeeded) throw paid.error + return { answer: 'answer q1' } + } + if (questionId(request) === 'q2') throw new Error(bridge) + throw new Error('DSPy RLM bridge returned no answer') + }, { + pricing: { inputUsdPerMillion: 1.25, outputUsdPerMillion: 10 }, + max_output_tokens: 8_192, + max_reasoning_tokens: 32_768, + }) + const result = await runTraceQuestions({ + questions: ['Which commands ran?', 'Which failed?', 'What was asked?'].map((question) => ({ question })), + spans: fixtureSpans(), + engine, + concurrency: 1, + budgetUsd: 1, + }) + + // The ledger is exhausted for both failures ($0.70 settled against a $0.41 + // reservation), but each failure names its own cause, so neither is + // relabelled: the bridge one keeps the kind the CLI's reinstall hint reads. + expect(result.questions.map((answer) => answer.failure?.kind)).toEqual([undefined, 'error', 'no-answer']) + expect(result.questions[1]!.failure?.message).toContain('could not start') + }) + it('nests an answer\'s own headings under its question section, leaving fenced text alone', async () => { const answer = [ '# traces ask', @@ -434,6 +472,19 @@ describe('runTraceQuestions', () => { expect(result.report).toContain('```sh\n# not a heading\n```') }) + it('keeps a multi-line question on one heading line', async () => { + const question = 'Which shell commands ran?\nName each one and its exit code.' + const { engine } = scriptedEngine(async () => ({ answer: 'one command ran' })) + const result = await runTraceQuestions({ + questions: [{ id: 'commands', question }], + spans: fixtureSpans(), + engine, + }) + // A heading is one line; the answers file keeps the question verbatim. + expect(result.report).toContain('## commands: Which shell commands ran? Name each one and its exit code.\n') + expect(result.questions[0]!.question).toBe(question) + }) + it('reports the effective concurrency next to the requested one', async () => { const { engine } = scriptedEngine(async () => ({ answer: 'one worker was enough' })) const result = await runTraceQuestions({ @@ -467,6 +518,25 @@ describe('runTraceQuestions', () => { expect(result.report).toContain(`**Unresolved:** trace://${TRACE}/span/invented-span`) }) + it('answers a question whose citation the model wrote in bold', async () => { + const { engine } = scriptedEngine(async () => ({ + answer: `The session ran one command (**trace://${TRACE}/span/tool-1**).`, + })) + const result = await runTraceQuestions({ + questions: [{ id: 'commands', question: 'Which shell commands ran?' }], + spans: fixtureSpans(), + engine, + }) + const [answer] = result.questions + expect(answer!.citations).toEqual([ + { uri: `trace://${TRACE}/span/tool-1`, traceId: TRACE, spanId: 'tool-1', resolved: true }, + ]) + expect(answer!.failure).toBeUndefined() + expect(answer!.status).toBe('answered') + // The exit code follows `ok`, so the formatting must not decide it. + expect(result.ok).toBe(true) + }) + it('fails an empty answer', async () => { const { engine } = scriptedEngine(async () => ({ answer: ' ' })) const result = await runTraceQuestions({ questions: [{ question: 'Anything?' }], spans: fixtureSpans(), engine }) @@ -603,6 +673,23 @@ describe('question input', () => { { uri: 'trace://t2/span/def', traceId: 't2', spanId: 'def' }, ]) }) + + it('extracts a citation the model wrapped in Markdown emphasis', () => { + // A model emphasises a citation as readily as it writes one bare; keeping + // the closing delimiters in the span ID fails a correct answer. + expect(traceCitationsInText('**trace://t/span/abc**, _trace://t/span/def_, ~~trace://t/span/ghi~~.')) + .toEqual([ + { uri: 'trace://t/span/abc', traceId: 't', spanId: 'abc' }, + { uri: 'trace://t/span/def', traceId: 't', spanId: 'def' }, + { uri: 'trace://t/span/ghi', traceId: 't', spanId: 'ghi' }, + ]) + // Emphasis and sentence punctuation in either order. + expect(traceCitationsInText('ran it (**trace://t/span/abc**). Then **trace://t/span/def.**')) + .toEqual([ + { uri: 'trace://t/span/abc', traceId: 't', spanId: 'abc' }, + { uri: 'trace://t/span/def', traceId: 't', spanId: 'def' }, + ]) + }) }) describe('answer schemas', () => { @@ -612,6 +699,19 @@ describe('answer schemas', () => { .toThrow('answerSchema.properties.n: unsupported JSON Schema keyword "minimum"') }) + it('rejects a type-specific keyword whose type is not declared', () => { + // Without "type": "object" the constraint is skipped for every non-object + // answer, so `5` would pass a schema that demands a property. + expect(() => assertAnswerSchema({ required: ['a'] })) + .toThrow('answerSchema: "required" is checked only for an object; declare "type": "object" alongside it') + expect(() => assertAnswerSchema({ type: 'array', items: { properties: { a: { type: 'string' } } } })) + .toThrow('answerSchema.items: "properties" is checked only for an object') + expect(() => assertAnswerSchema({ items: { type: 'string' } })) + .toThrow('answerSchema: "items" is checked only for an array; declare "type": "array" alongside it') + // A nullable object still declares the type it constrains. + expect(() => assertAnswerSchema({ type: ['object', 'null'], required: ['a'] })).not.toThrow() + }) + it('checks types, required properties, enums, and array items', () => { const schema = { type: 'array', diff --git a/tests/finding-rejections.test.ts b/tests/finding-rejections.test.ts index 7221f26..3497e4d 100644 --- a/tests/finding-rejections.test.ts +++ b/tests/finding-rejections.test.ts @@ -22,11 +22,26 @@ describe('finding rejections', () => { uri: 'trace://t/span/s', reason: 'trace span does not exist', })).toBe('trace span does not exist; uri trace://t/span/s') + // The kind is already in the message; repeating it would print + // "finding rejected: insufficient evidence citations — insufficient evidence citations". expect(findingRejectionDetail('finding rejected: insufficient evidence citations', { required: 2, distinct: 1 })) - .toBe('insufficient evidence citations; 1 of 2 required distinct citation(s)') + .toBe('1 of 2 required distinct citation(s)') expect(findingRejectionDetail('[analyst] ok failure-mode', {})).toBeUndefined() }) + it('names the cause of every rejection kind that carries one', () => { + // The dspy engine's bridge-row rejection puts its cause in `reason`. + expect(findingRejectionDetail('finding rejected: bridge row failed schema validation', { + reason: 'invalid_type at findings.0.severity: expected string', + })).toBe('invalid_type at findings.0.severity: expected string') + // The kind factory's schema failure puts its cause in `issues`. + expect(findingRejectionDetail('[improvement] finding rejected: schema failure', { + issues: ['claim: required', 'confidence: expected number'], + })).toBe('claim: required; confidence: expected number') + // A kind that carries no cause adds nothing: the caller prints the message alone. + expect(findingRejectionDetail('[improvement] finding rejected: schema failure', {})).toBe('') + }) + it('counts per analyst, with a default for unprefixed events', () => { const tally = createFindingRejectionTally('q1') tally.record('finding rejected: unresolved evidence', { reason: 'trace span does not exist' })