feat(ask): answer free-form questions concurrently under one budget - #108
feat(ask): answer free-form questions concurrently under one budget#108drewstone wants to merge 3 commits into
Conversation
`traces ask` and the exported `runTraceQuestions` run one or many questions over one or many sessions. Each question is its own `runTraceAnalyst` call, so the engine's prose answer survives; the analyst registry keeps only findings and runs analysts one at a time. - Questions run concurrently through a worker pool with a configurable limit. Unlike the import pool, one failed question never stops the others. - One shared `CostLedger` bounds the whole run, so `--budget` means the same thing whatever the number of questions; `--question-budget` bounds one question. A budget below one call's reservation refuses the run before any model call, and a budget that serializes the pool warns instead. - Every `trace://` citation in an answer is resolved against the store; an unresolvable citation fails that question. - A question may fix its answer's shape with a small JSON Schema subset. An unsupported keyword is rejected rather than ignored. - The question layout fits the DSPy input preview: the question stays whole and the answer rules sit in the first 500 characters of the instructions. - Output is `answers.json` and `report.md`, written before the exit code is decided; exit 1 when any question failed. Also surface what the evidence gate refused, and stop reporting a failed external analyzer as success: - `finding rejected` log lines now name the reason and the offending URI, and rejection counts by reason reach the analyst table's Detail cell, `TraceInvestigationResult.findingRejections`, and the ask JSON. - `analyze` exits 1 when a requested `--analyzer` fails, after writing the report that holds its error. The `--llm` engine construction moves to `analysisEngineFromEnv` in `analyst-model-call.ts`, and the shared trace-file and store setup to `analysis-store.ts`, so `ask` reuses both instead of duplicating them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
drewstone
left a comment
There was a problem hiding this comment.
Adversarial review — T1 / T2 / T7
I checked out origin/feat/ask-questions (55044df) in a clean worktree, read the whole diff, and ran pnpm install --frozen-lockfile, check:source, typecheck, test, build, check:package locally. All green (828 tests, 59 files), and both CI jobs pass.
I verified each added test fails against origin/main's src/ by copying the changed test files onto a main worktree that shares this branch's node_modules:
| Test | On main |
|---|---|
analyze --analyzer false exits 1 |
fails: analyze exited 0 although its requested analyzer failed |
ask needs at least one question |
fails: ask exited 0 without a question |
ask writes artifacts then exits 1 |
fails: expected '' to contain '# traces ask' |
runTraceInvestigation counts gate rejections |
fails: findingRejections is undefined |
analystRunDetail names rejection reasons |
fails: expected '—' |
tests/ask.test.ts imports ../src/ask.js, so it cannot exist on main at all. No test passes vacuously.
Privacy: clean. Every fixture is synthetic and generic (git status --short && pnpm test, 12 passed, Running the checks now.). No absolute paths, home directories, rollout names, session UUIDs, or keys anywhere in the diff.
Reuse: clean. analysisEngineFromEnv genuinely replaces the duplicated --llm construction rather than adding a second one, and analysis-store.ts collapses the four copies of write-file-then-open-store. The repo carries no JSON Schema library (deps are the four @tangle-network/* packages plus sandbox), so the answer-schema.ts subset is not re-implementing something already present. traceCitationsInText is a parser; the existing spanEvidenceUri is only a builder.
Unknown shapes are counted, not guessed. I checked every finding rejected: emission in agent-eval 0.179.0 (kind-factory schema failure / subject / insufficient citations / rejectEvidence, plus dspy-rlm-engine's bridge-row rejection). All five are matched by REJECTION_MESSAGE, unresolved evidence correctly defers to fields.reason, and the registry's [<id>] prefix and field pass-through are preserved. preparedContext reports omitted_traces rather than silently truncating. Per-question cost attribution is sound: runTraceAnalyst builds costTags = { analystId, analystRunId: correlationId } and settles the receipt through that filter, so with id: ask.<qid> and correlationId: <runId>:<qid> each question's usage is its own. I also confirmed the DSPy engine really threads request.costLedger into startExternalOptimizerModelProxy, so the shared-budget claim holds against the real engine and not only the fake one, and that executionConfig really carries pricing.outputUsdPerMillion, max_output_tokens, and max_reasoning_tokens — engineCallReservationFloorUsd is live in production, not dead code that only the hand-written test config satisfies. OtlpFileTraceStore shares its index through createSharedAbortableTask, so one store across concurrent questions is safe.
No regression found in other adapters, report formats, the evidence JSONL schema, or the improvement artifacts: findingRejections is an additive optional field on TraceInvestigationResult, TraceEvidenceRow is untouched, and analystRunDetail gained an optional second parameter.
Blocking
1. Ctrl-C on a run whose answers carry citations throws away every answer
src/ask.ts — verifyCitations sits outside askOne's try/catch and forwards the run's AbortSignal to store.hasSpans, which begins with context?.signal?.throwIfAborted(). When the signal is aborted after an answer came back, askOne rejects, Promise.all rejects, and runTraceQuestions throws. cmdAsk then never reaches writeTraceQuestionsArtifacts, so no answers.json and no report.md are written, and the answers already paid for are gone.
Reproduced with a scripted engine on the branch (three questions, concurrency: 1, abort fired in a microtask after the first answer returns):
answer contains trace://<t>/span/tool-1 → THREW Error: ctrl-c
answer contains no trace:// URI → OK [["q1","answered",null],["q2","failed","aborted"],["q3","failed","aborted"]]
The only difference is whether the answer cites a span, which pins the escape to verifyCitations.
This contradicts what the PR and the docs promise for exactly the path cmdAsk installs a SIGINT handler to serve:
- PR body: "Both are written before the exit code is decided"
docs/trace-analysts.md: "The artifacts are written before the exit code is decided" and "A failed question never stops the others. Its failure is recorded on its own answer, and the remaining answers are written."
Interrupting a long concurrent run is the case where an operator most wants the partial answers already bought. Suggested fix, smallest first:
- Drop the signal from citation verification — it is an in-memory lookup against an index that is already built, so it does not need to be cancellable:
await store.hasSpans({ trace_id, span_ids }). - Wrap the whole post-engine block (
verifyCitations, schema parse) in atry/catchthat records{ kind: 'aborted' | 'error' }on that answer, so no later addition to that block can escape either. - In
cmdAsk, write whatever artifacts exist even whenrunTraceQuestionsthrows, so the guarantee holds at the command level too.
A test in the style of the existing ones: abort after the first answer, assert three recorded answers (answered, aborted, aborted) and that both artifact files exist.
Non-blocking
2. budget-refused is only proven on the in-process path, and the real path will not reach it. isBudgetRefusal matches /would exceed ceiling|model cost limit reached/ on the message text. The test drives it by having the fake engine rethrow the ledger's own runPaidCall error inside Node. In a real run the refusal happens inside the model proxy, and this PR's own A2 note says the bridge's raise_for_status hides the Node error text — so shared-ledger exhaustion will most likely land as failed: error with a raise_for_status message. Exit code and retained answers are still right, but the report's failure taxonomy misleads in the one scenario --budget exists for. Either say so where the failure kinds are documented, or reconcile the kind against the ledger at failure time (if budgetUsd - settled spend < engineCallReservationFloorUsd, call it budget-refused whatever the message says).
3. The exit-code change is wider than the release note. assertRequestedAnalysesRan also runs in cmdInvestigate and cmdImprove, and externalFailureMessage sees result.external, which mergeTracesConfig fills from config.externalAnalyzers as well as --analyzer. investigate and improve load a default config file (analyze passes loadDefaultConfig: false), so a repo whose default traces config declares a flaky external analyzer now sees those two commands flip to exit 1. That is defensible behavior, but the PR body and docs/trace-analysts.md both say only "analyze exits 1 when a requested --analyzer fails". Name investigate, improve, and config-declared analyzers in the release note.
4. totals.wallTimeMs excludes setup. runStarted is taken after writeAnalysisTraceFile and openAgenticTraceStore, so the stderr line wall N s and the report's "Wall time" omit writing and indexing the OTLP file. On a 48 MB session that is not free, and wall time is the metric this command exists to improve. Either start the clock at the top of runTraceQuestions or report setup separately.
5. result.concurrency is the requested value, not the effective one. With one question and --concurrency 8 the report reads concurrency 8 while only one worker ever exists (effectiveConcurrency = min(concurrency, questions.length)). peakConcurrency carries the truth, but the summary line does not. Cheap to report both.
6. Default IDs collide with explicit ones. IDs default to q<index+1> over the combined file-then-flags list, so a file entry with "id": "q2" plus one positional --question both resolve to q2 and the run dies with duplicate question ID "q2" before anything runs. Assign defaults after explicit IDs are reserved, or namespace them.
7. Doc surfaces that still omit ask. --help was updated but the README flag table's --otlp row still reads "validate, analyze, investigate, improve, stream", and --source-bundle is absent from that table although collectSpans now accepts it for ask. The two shipped agent skills (skills/inspect-agent-traces/SKILL.md, skills/build-trace-analyst/SKILL.md) enumerate the command set and do not mention ask — they are what an agent reads to pick a command.
8. No end-to-end CLI test of a successful ask. Both tests/cli.test.ts cases are failure paths (no question; dead bridge). Artifact writing is covered by the dead-bridge case, but the exit-0 branch of cmdAsk is not exercised anywhere. The in-process tests cover success, so this is a gap rather than a hole — a scripted-engine CLI case would close it.
9. Two cosmetic report issues. analystRunDetail appends the rejection text after condenseAnalystError(raw, ANALYST_DETAIL_MAX_CHARS), so the Detail cell can now exceed the length that constant names. And answer text is embedded raw under ## <id>: <question> in report.md; I confirmed an answer of # traces ask plus a Markdown table renders as a second header and table inside the report. Neither is a security issue, but the per-question section has no delimiter.
Everything except item 1 is fine to land as follow-up. Item 1 is a few lines and has a direct test.
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) <noreply@anthropic.com>
|
Repaired in 1 — Ctrl-C discarded every answer. Confirmed and fixed exactly as suggested.
The test is the one you described: three questions, 2 — 3 — exit-code scope. Named in both the PR body and the docs: the check runs on 4 — wall time. The clock starts before 5 — effective concurrency. 6 — ID collisions. Explicit IDs are reserved before any default is assigned, and a default keeps its own position where it can. A file entry 7 — doc surfaces. The 9 — report cosmetics. 8 — no CLI test of a successful Local check set green: |
drewstone
left a comment
There was a problem hiding this comment.
Adversarial review — PR #108 (feat/ask-questions, head 185f975)
Reviewed in a fresh worktree off origin/feat/ask-questions. I ran the full check set myself and re-ran the new tests against the pre-change source to confirm they are load-bearing.
Checks
| Check | Result |
|---|---|
pnpm install --frozen-lockfile |
ok |
pnpm check:source |
ok |
pnpm typecheck |
ok |
pnpm test |
833 passed, 59 files |
pnpm build |
ok |
pnpm check:package |
ok |
gh pr checks |
CI (Node 22.13.0) pass, CI (Node 24.18.0) pass |
git merge-tree origin/main HEAD |
no conflicts |
Do the tests fail without the change?
Yes, everywhere I could isolate it.
tests/ask.test.tsagainst55044df'ssrc/ask.ts(the pre-repair commit): 6 red — overlap/setupTimeMs, the Ctrl-C abort case, the ledger-reconciledbudget-refused, heading nesting,effectiveConcurrency, and explicit-ID reservation. Each repair item in the PR body has a test that is red before it and green after.tests/report.test.ts,tests/improvement.test.ts,tests/cli.test.tsagainstorigin/main'ssrc/: 6 red — bothanalystRunDetailcases, thefindingRejectionsinvestigation case,analyze --analyzer falseexit 1, and bothtraces askCLI cases.
The concurrency test is a real test, not a timing coincidence: deferred gates prove exactly three of five questions are in flight and that the fourth starts only when one finishes, and a separate case measures wall time under half the summed question time.
Privacy
Clean. Every fixture is inline synthetic JSONL in the existing style (git status --short && pnpm test, session-ask, trace-ask). No path, session id, hex rollout id, or transcript text from this machine appears anywhere in the diff. The only URL added is the pre-existing router.tangle.tools constant, moved out of cli.ts.
Regressions I looked for and did not find
- Adapters are untouched. Evidence JSONL schema is untouched;
findingRejectionsis an additive optional field onTraceInvestigationResultandReportMeta. - Report format:
analystRunDetailkeeps the same cell bound when a rejection summary shares it (tested with a 400-character reason), and the Detail cell is still—when both parts are empty. - Store sharing under concurrency is safe:
BufferedOtlpTraceStorememoizes the index behindindexTask/indexValue, andensureIndexed()is awaited once before the pool starts, so every question's tool call is an in-memory lookup over one shared index.createDspyRlmTraceEngine'sanalyze()builds its own tool callback, model proxy, and Python process per call, so one engine object driving N concurrent questions has no shared mutable state. - Ledger accounting is correct:
summary({ channel: 'analyst' })matches the channel the dspy engine actually books under,totalCallscounts settled receipts only sototalCalls + pendingCallsdoes not double-count, and per-questionusagecomes fromrunTraceAnalyst's tag-filtered settlement, which isolates questions from each other. - Unknown counts are counted, not guessed:
modelCalls/toolCallscollapse tonullviasumOrNullif any question's count is unknown, cost provenance keepsuncapturedwith a null amount,omitted_tracescounts traces pastMAX_CONTEXT_TRACES, and unparseable span times givenull, not 0. - Duplication:
analysis-store.tsgenuinely removes the second copy of the store construction (nonew OtlpFileTraceStoreorGENERATED_TRACE_FILE_CEILINGremains outside it), andanalysisEngineFromEnvremoves the second engine construction as the brief asked.
The investigate/improve exit-code widening to config-declared externalAnalyzers is a real behavior change, but it is deliberate, documented in docs/trace-analysts.md, and ok: false on an ExternalAnalysisResult only ever means a genuine failure (runExternalAnalyzers never sets it for a skip).
Blocking
1. A correct citation written in Markdown emphasis is scored as unresolved, and the question fails.
src/ask.ts — TRACE_URI excludes backticks, brackets, and parentheses from the span-id character class, and the trailing strip is /[.:!?]+$/. It does not cover *, _, or ~, which are exactly the characters a model puts around a citation when it emphasises it.
"bold **trace://t/span/abc**" -> "trace://t/span/abc**"
"emph _trace://t/span/abc_" -> "trace://t/span/abc_"
"tilde ~~trace://t/span/abc~~" -> "trace://t/span/abc~~"
Reproduced end-to-end through runTraceQuestions with the synthetic fixture: an answer reading The session ran one command (**trace://trace-bold/span/tool-1**). — where tool-1 is in the trace — comes back as
status failed
citation {"uri":"trace://trace-bold/span/tool-1**","spanId":"tool-1**","resolved":false}
failure {"kind":"unresolved-citations","message":"1 cited span(s) do not exist: ..."}
so result.ok is false and ask exits 1 on a correct answer. This is the headline guarantee of T1 ("every trace:// citation is checked") returning a wrong verdict, and it drives the command's exit code. The answer text survives in answers.json, so it fails loudly rather than silently — but an operator scripting on the exit code gets a false negative, and the rules the model is given say nothing about formatting.
Suggested fix: extend the trailing strip to the Markdown emphasis run (/[.,:;!?*_~]+$/), or resolve once as-matched and retry with emphasis stripped before declaring a citation unresolved. Add a case to the existing traceCitationsInText test covering **…**, _…_, and ~~…~~ — it is red today.
Non-blocking
2. findingRejectionDetail drops the field that names the cause for two of the five rejection kinds, and echoes itself for a third.
fields.reason is read only when the message is unresolved evidence. The dspy engine's own finding rejected: bridge row failed schema validation carries the cause in fields.reason (<code> at <path>: <message>) and finding rejected: schema failure carries it in fields.issues; both are discarded, and the CLI line becomes finding rejected: schema failure — schema failure. docs/trace-analysts.md says "the CLI log prints the reason and the offending URI on each finding rejected line", which is only true for one kind. Cheap fix: fall back to fields.reason for any kind, add issues, and suppress the — suffix when the detail equals the message.
3. The ledger reconciliation can relabel a genuine engine failure as budget-refused.
ledgerIsExhausted returns true whenever budgetUsd - settled < floor at the moment a question failed, whatever the cause. A bridge-version mismatch on the last question of a nearly-exhausted budget is reported as budget-refused — and cmdAsk's bridge hint only fires for failure.kind === 'error', so the actionable "install agent-eval-rpc[dspy]==<v>" line is suppressed exactly when it is needed. Consider checking isBridgeMismatchError before the ledger reconciliation, or emitting the hint on any kind whose message matches.
4. no-answer is unreachable through the real engine.
parseBridgeOutput throws DSPy RLM bridge returned no answer when answer is empty, so runTraceAnalyst rejects and the kind is error; only the fake engine can produce no-answer. Either map that message to no-answer, or drop the kind so the documented list matches what can actually occur.
5. The missing-key error arrives after the session is parsed.
cmdAsk calls collectSpans before analysisEngineFromEnv, so an operator with no TANGLE_API_KEY waits through a large session's adapter pass to be told the key is missing. Building the engine before collectSpans costs nothing and fails in a second.
6. A question containing a newline breaks the report heading.
normalizeTraceQuestions trims but does not reject internal newlines, and renderTraceQuestionsReport emits ## ${id}: ${question} raw. Collapse whitespace in the heading, or reject a multi-line question with the same message style as the length check.
7. The pool's failure path returns an array other workers are still writing into.
On poolError, wallTimeMs is taken and the holes are filled while the remaining workers are still awaiting askOne; their later answers[index] = … mutates the array already handed to the caller. Unreachable today because askOne catches everything, but that is the case this branch exists for. Promise.allSettled over the workers would make the branch safe on its own terms.
8. answerSchemaErrors skips required when the value is not an object.
{ "required": ["a"] } with no "type": "object" accepts the answer 5 with no error. Either require a type alongside required in assertAnswerSchema, or report a non-object against a schema carrying required.
9. The trace://…/span/… parse duplicates agent-eval's parseTraceSpanEvidenceUri.
Same regex, same decodeURIComponent and try/catch, but it is internal to kind-factory and not exported. Nothing to do here; it belongs on the PR's existing agent-eval follow-up list next to A1/A2/A3.
10. ask prints the whole report to stdout even with --dir.
improve prints a one-line pointer to the artifact directory. On a many-question run with long answers this is a lot of scrollback that is already in report.md.
11. An unknown command still prints usage and exits 0.
default: usage() in main. That is the other half of the brief's F8 bullet that motivated ask; out of scope for T1 as written, but it is one line and the same class of "exit 0 that means failure" this PR is otherwise closing.
Item 8 in the PR body (no end-to-end CLI test of a successful ask) — the argument is sound: createDspyRlmTraceEngine asserts parsed.modelCalls === modelProxy.successfulCompletions() and then modelProxy.assertExecutionComplete(), so a synthetic bridge cannot report success without driving the real proxy. I would not hold the PR for it, and I would not add an environment-guarded live-provider test to the suite either.
Fix item 1 and this is good to merge.
`traceCitationsInText` trimmed only `.:!?` from a matched URI, and the
span-id character class admits `*`, `_` and `~`. An answer that cited a
real span as `**trace://<t>/span/<s>**` 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) <noreply@anthropic.com>
|
Repaired in 1 — a correct citation in Markdown emphasis scored as unresolved. Confirmed and fixed at the parser. The trailing strip is now Two tests, both red before the change:
I did not add a "cite bare, without emphasis" line to 2 — 3 — the ledger reconciliation relabelling a genuine failure. Fixed by ordering, in one 4 — 5 — engine built after 6 — a multi-line question breaking the report heading. The heading folds the question's whitespace onto one line; 7 — the 8 — Not changed, with reasons:
Local check set green on |
What this fixes
Brief classes F8 (plumbing) and F6 (the evidence gate hides its rejections), items T1, T2, and T7.
Today a reading subagent beats the CLI partly because the CLI throws work away. The registry keeps only findings, so the engine's prose answer is discarded; analysts run one at a time; there is no way to ask a question of your own, and an unknown command exits 0. A refused finding is reported only as a log line whose reason never reaches the operator, so "0 findings" and "the gate refused everything" look identical. A failed external analyzer writes its error into the report and exits 0.
T1 —
traces askandrunTraceQuestionsOne or many free-form questions over one or many sessions. Each question is its own
runTraceAnalystcall, so the answer text survives.CostLedgerbounds the whole run.--question-budgetis the provider ceiling for one question. A budget below one model call's reservation refuses the run before any model call; a budget that admits fewer concurrent reservations than--concurrencyruns with a warning naming how many it covers.trace://<trace_id>/span/<span_id>URI in an answer is resolved against the store. An unresolvable citation fails that question.answers.json(answers, parsed answers, citations with resolution, accepted findings, rejections by reason, model calls, tool calls, cost withobserved/estimated/uncapturedprovenance, latency) andreport.md. Both are written before the exit code is decided;askexits 1 when any question failed, returned no answer, broke its schema, or cited a missing span. Ctrl-C is one of those failures, not an exception: the answers already bought are kept, the questions the run never reached are recorded asaborted, and both artifacts are still written.The
--llmengine construction moves out ofcli.tsintoanalysisEngineFromEnv(analyst-model-call.ts), and the shared trace-file plus store setup intoanalysis-store.ts.askand--llmnow build the same engine from one place instead of two.T2 — gate rejections visible, external failures fail
finding rejectedlog lines print the reason and the offending URI.TraceInvestigationResult.findingRejections, and the ask JSON, so a zero finding count can be told apart from a gate that refused everything.analyze,investigate, andimproveexit 1 when a requested analyzer (halo, hodoscope, prime, or a command) fails, after writing the report that holds its error.Behavior change: the external-analyzer exit code, on all three commands.
assertRequestedAnalysesRancovers every analyzer the run requested, not only the ones named on the command line:investigateandimproveload the default traces config, so an analyzer declared in itsexternalAnalyzerscounts as requested and a flaky one turns those two commands red.analyzepassesloadDefaultConfig: false, so only its own--analyzerflags reach the check. A script that treated exit 0 as "the analyzer ran" was reading a report that said otherwise, but it will now see a failure. It belongs in the release notes.T7 — question layout for the DSPy preview
The engine shows the model the first 500 and last 500 characters of a long input. So the question field stays under the preview size and reaches the model whole (an over-long question is rejected before any model call, naming the limit), and the answer rules sit in the first 500 characters of the instructions. Detail goes in the entry's
instructions, which the model reads after the rules.Tests
All fixtures are synthetic, written inline in the style of the existing tests. A scripted fake engine implements the engine contract and calls the real tool handlers it receives.
tests/ask.test.tsanswers.json; citations resolve; the report contains the answer.--concurrency 3, deferred promises prove exactly three run at once and the fourth starts only when one finishes.budget-refusedfailures, with total spend at or below the ceiling.unresolved-citations; an empty answer fails asno-answer.tests/cli.test.tsanalyze --analyzer falsewrites the report, then exits 1 naming the analyzer.askwith three questions against a fakeTRACES_PYTHONbridge that dies at startup: exit 1, both artifacts written, every question recorded with its failure and the bridge's reason.tests/improvement.test.ts,tests/report.test.tsGreen locally:
pnpm check:source,pnpm typecheck,pnpm test(828 tests, 59 files),pnpm build,pnpm check:package.Docs
traces askis documented in README.md (Contents, CLI reference, flag table, and an "Ask questions" section) and in docs/trace-analysts.md (questions and the preview limit, answer schemas, the run's guarantees, budget under concurrency, an SDK example, gate rejections, and the external-analyzer exit code).Follow-up that needs agent-eval
Nothing here is blocked on agent-eval, but three items would remove workarounds this PR carries:
analyze --llmwould get the same benefits without a separate command, andaskcould drop its own pool.searchTracereturns escaped raw JSON, so a correctly copied excerpt is refused. This PR only makes the refusal visible; T3 (PR fix(analyst): normalize model citations before the evidence gate #107) normalizes it inpostProcess, and A2 would fix it at the owner. Passing the Node error text through the bridge would also stopraise_for_statusfrom hiding causes, which is why one failure kind here is matched by message text rather than by class.searchTracehits carry no time fields and cap at 500 matches with no cursor, so enumeration and "last X" questions still cannot be answered exhaustively however the questions are asked.Review repairs (185f975)
The blocking finding is fixed, with a test that fails against the previous commit, and every non-blocking item except one is addressed.
Blocking — Ctrl-C discarded every answer. All three suggested steps:
verifyCitationstakes no signal. The store index is built byopenAgenticTraceStorebefore the first question runs, so it is an in-memory lookup and need not be cancellable.try/catchthat records{ kind: 'aborted' | 'error' }on that answer, so no later addition can escape either.warnings, andrunTraceQuestionsstill returns.cmdAsktherefore reacheswriteTraceQuestionsArtifactson every path where an answer exists, and decides the exit code from the written result.New test, in the existing style: abort in a microtask after the first answer returns, with a citation in that answer. It asserts
[['q1','answered'],['q2','failed','aborted'],['q3','failed','aborted']], that the kept answer's citation resolved, and that both artifact files exist. Restoring only the signal onstore.hasSpansturns it red.Non-blocking, fixed:
budget-refusedon the real path. The kind is now reconciled against the ledger: a failed question isbudget-refusedwheneverbudgetUsd - settled analyst spend < engineCallReservationFloorUsd, whatever the message says, and the original message is kept verbatim on the answer. Tested with an engine that spends $0.70 of a $1 budget and then fails with a bridge-shapedHTTPErrorthat names no ceiling. Documented where the failure kinds are.docs/trace-analysts.mdnow nameinvestigate,improve, and config-declaredexternalAnalyzers, and say whyanalyzediffers.writeAnalysisTraceFile, andtotals.setupTimeMsnames the writing-and-indexing part. Both appear in the report line and inanswers.json.result.effectiveConcurrencyismin(concurrency, questions); the summary line readsconcurrency 8 (1 effective)when they differ.q2plus one positional question now yieldsq2, q3;q1explicit after one default yieldsq2, q1.--otlprow listsask,--source-bundlehas its own row, and both shipped skills mentionask.inspect-agent-tracesis capped at 5,000 bytes bytests/skills.test.ts, so the new section is paid for by tightening prose in place; no instruction was dropped.analystRunDetailnow budgets the whole cell, so a rejection summary cannot push it past the lengthANALYST_DETAIL_MAX_CHARSnames (tested with a 400-character reason). Answer text embedded inreport.mdhas its own Markdown headings demoted below the question's##section, with fenced blocks left alone, so an answer cannot reflow the report's outline.Non-blocking, not fixed — 8, no end-to-end CLI test of a successful
ask. It cannot be closed offline.createDspyRlmTraceEngineassertsparsed.modelCalls === modelProxy.successfulCompletions()and then callsmodelProxy.assertExecutionComplete(), so a syntheticTRACES_PYTHONbridge cannot report a successful run without actually driving the model proxy, which forwards to the configured provider. A CLI success case therefore needs a live provider call, which the test suite must not make. The in-process tests cover the success path throughrunTraceQuestions, which is where the answer, citation, budget, and artifact logic lives;cmdAsk's remaining exit-0 branch is the artifact write plus the two stderr lines. Happy to add it behind an opt-in environment guard if you would rather have it than not.Local check set green on
185f975:pnpm check:source,pnpm typecheck,pnpm test(833 tests, 59 files),pnpm build,pnpm check:package.Second review repairs (541e2eb)
Blocking — a correct citation in Markdown emphasis scored as unresolved.
traceCitationsInTexttrimmed only.:!?, while the span-id class admits*,_and~, so**trace://<t>/span/<s>**carried its closing delimiters into the span ID, resolved against nothing, and failed the question withunresolved-citations—askexited 1 on a correct answer. The trailing strip is now/[.,:;!?*_~]+$/, covering emphasis and sentence punctuation in either order. Two tests, both red before the change: the unit case ontraceCitationsInTextfor**…**,_…_and~~…~~, and an end-to-end case throughrunTraceQuestionswhere a bold citation resolves andresult.okis true. The answer rules were left alone on purpose: they must fit the 500-character DSPy preview head, which has no room for a formatting sentence, and the parser is the right place for the fix.Non-blocking, fixed:
findingRejectionDetailreads the cause fromfields.reasonfor any kind and fromfields.issuesfor a schema failure, and omits a lead the message already carries, sofinding rejected: schema failure — schema failureis gone and the dspy engine's bridge-row rejection keeps its cause.analystLogprints the message alone when nothing is left to add. Documented.questionFailureKinddecides in order: aborted, a refusal that names itself, a bridge mismatch (stayserror), an empty bridge answer (no-answer), then the ledger fallback. The CLI's reinstall hint now fires on the message whatever the kind, so a mismatch on a nearly-exhausted budget keeps its actionable line.no-answerreachability. The bridge'sreturned no answermaps to the kind, which was otherwise unreachable through the real engine.collectSpans. A missingTANGLE_API_KEYis reported before a large session is parsed.answers.jsonkeeps it verbatim.Promise.allSettledover the workers, so a rejection cannot hand back an answers array the remaining workers still write into.assertAnswerSchemarejectsrequired,propertiesoradditionalPropertieswithout"type": "object", anditemswithout"type": "array";{ required: ['a'] }previously accepted the answer5.Not changed: the
parseTraceSpanEvidenceUriduplication is an agent-eval export request (follow-up list next to A1/A2/A3);askkeeps printing the report to stdout with--dir, because its output is the answer rather than an artifact pack, and the pointer stays on stderr; the unknown-command exit code stays with F8.Local check set green on
541e2eb:pnpm check:source,pnpm typecheck,pnpm test(839 tests, 59 files),pnpm build,pnpm check:package.🤖 Generated with Claude Code