fix(cli): surface the agent result in output and bad view - #137
fix(cli): surface the agent result in output and bad view#137AtelyPham wants to merge 2 commits into
Conversation
Empty completion message meant the answer lived only in goalVerification.evidence in report.json; CLI printed a bare "Goal achieved" and the viewer showed the same. buildVerdict now falls back to that evidence; cli-view derives an `answer`; viewer renders a "Result" panel (multi-line preserved, multi-task rows collapsed). +normalizeReport regression test.
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 3 (1 low, 2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 99.5s (2 bridge agents) |
| Total | 99.5s |
💰 Value — sound-with-nits
Surfaces the agent's actual answer (completion message, falling back to goal-verification evidence) in the CLI output, report verdict, and bad view viewer — a real UX fix; only nit is a small duplicated derivation.
- What it does: When a run succeeds but the agent's
completeaction leftagentResult.resultempty (its findings lived only ingoalVerification.evidence), the CLI printed a bare 'Goal achieved' and the viewer showed the same — forcing users tojqreport.json. This PR makes three layers surface the real answer: (1)buildVerdictin test-runner.ts:1065-1074 now derives `result?.trim() || evidence.join('\n') - Goals it achieves: The agent's user-visible answer reaches the user without grepping report.json — both in the terminal (single- and multi-task modes) and in the
bad viewHTML viewer. Particularly valuable for extract-heavy goals ('list the top 3', 'find the price') where the agent's answer is the entire point of the run but the completion message itself is often terse or empty while the substantive content sits i - Assessment: Good change on its merits. The problem is real and verified: runner.ts:2126/2146 just pass
action.resultthrough asagentResult.result, and that field is whatever the model typed as its completion message — frequently empty for extract goals where the answer was emitted as evidence via runner.ts:1825-2015 / execute-plan.ts:272. The fix lands in the right seams:buildVerdictalready owned ver - Better / existing approach: Searched src/ for existing answer-extraction helpers (
deriveAnswer,extractAnswer,getResult) — none exist. The one improvement worth flagging: the 'extract answer from agentResult' logic is now duplicated between test-runner.ts:1071-1074 (filter(Boolean)+ join) and cli-view.ts:231-234 (strict string-only filter + join). The two filters behave the same in practice because `GoalVerificatio - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound-with-nits
A focused, fully-wired fix that surfaces the agent's actual answer (which was already collected in goalVerification.evidence) into the CLI output, the report's verdict field, and the bad view viewer — instead of the bare 'Goal achieved' that previously forced users to jq the report.
- Integration: Every change site is on a live path.
buildVerdict(src/test-runner.ts:1060) is called at src/test-runner.ts:457 for every TestResult and its return value is stored as theverdictfield (test-runner.ts:469) — the canonical path for all test runs.normalizeReport(src/cli-view.ts:183) is called at src/cli-view.ts:409 bybad view.CliRenderer.testComplete(src/cli-ui.ts:182) is wired via th - Fit with existing patterns: Fits the codebase's grain. The fallback chain
result || evidence.join('\n') || 'Goal achieved'(test-runner.ts:1072-1074) is a direct generalization of the priorresult || 'Goal achieved'.normalizeReportalready exists to reshape report JSON for the viewer (cli-view.ts:183-254), so deriving ananswerfield there is in-pattern, not a new surface. Theverdictfield is preserved alongside - Real-world viability: Robust on the edge cases I checked. Both code paths guard undefined (
?? []), non-array evidence (Array.isArray ? ... : []), non-string entries (.filter((e): e is string => ...)), and whitespace-only results (.trim()). Multi-line verdict is handled in the CLI: per-line indent for single-task (cli-ui.ts:204), newline-collapse + 80-char truncate for multi-task rows (cli-ui.ts:208-209). I veri - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
🔎 Heuristic Signals
🟡 Cruft: console debug added src/cli-ui.ts
console.log(resultText.split('\n').map(line => ` ${line}`).join('\n'))
🎯 Usefulness Audit
🟡 'result → evidence → default' fallback duplicated across test-runner and cli-view [ergonomics] ``
The same three-step fallback (agentResult.result → goalVerification.evidence.join('\n') → 'Goal achieved'/undefined) now exists at src/test-runner.ts:1071-1074 (writes
verdictgoing forward) and src/cli-view.ts:231-234 (derivesanswerfor any report, including old ones). The duplication is intentional — the second is a backfill for historical reports whose verdict was already baked as 'Goal achieved' — but a tiny shared helper likederiveAgentAnswer(agentResult)would keep the two paths fr
💰 Value Audit
🟡 Answer derivation duplicated in buildVerdict and normalizeReport [duplication] ``
test-runner.ts:1071-1074 and cli-view.ts:231-234 both compute
result?.trim() || evidence.join('\n') || fallbackfrom the sameagentResult. The filter predicates differ (filter(Boolean)vs(e): e is string => typeof e === 'string' && e.trim().length > 0) — equivalent today only becauseevidenceis typedstring[](types/result.ts:37). Extract a singlederiveAnswer(agentResult)helper (next toGoalVerificationin types/result.ts, or in cli-view.ts and imported by test-runner.ts) so
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
✅ No Blockers —
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Approved — 8 non-blocking findings — 98b57d7c
Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision.
Full immutable report for this review: trace
Summary comment for this run: full summary
tangletools · 2026-07-27T15:24:46Z · immutable trace
Address review nits on #137: - Extract shared deriveAnswer() used by buildVerdict + normalizeReport — removes the duplicated result->evidence->default derivation and gives a directly unit-tested seam. - Guard non-string `result` via typeof so a malformed report.json can't crash the viewer. - cli-ui: color multi-line results per line; strip trailing newline so it doesn't emit a stray indented blank row. - Backfill `answer` on the single-agent report shape so its viewer Result panel renders too. - Tests: deriveAnswer branches (+ whitespace/non-string evidence) and the multi-line cli-ui render paths.
|
Addressed the review findings in
Skipped — cap verdict length: intentional. Surfacing the full answer is the point of this change; the length-constrained sinks already bound their own output (junit slices to 200, fan-out truncates to 2000), while the terminal and Gate green on |
Problem
When a run's completion message (
agentResult.result) is empty, the agent's actual answer was written only togoalVerification.evidenceinreport.json. The CLI printed a bare "Goal achieved" and thebad viewviewer showed the same — so the user had tojqthe report to find the result.Fix
test-runner.tsbuildVerdict— when the completion message is empty, fall back togoalVerification.evidence(joined). This surfaces the answer in the CLI output and the report'sverdictfield, instead of a bare "Goal achieved".cli-ui.ts— indent every line so a multi-line result stays aligned under the ✓/✗ line (single-task); collapse newlines before truncating multi-task rows so they stay single-line.cli-view.tsnormalizeReport— derive ananswerfield (completion message → else evidence) so the viewer shows the real result even for older reports whoseverdictwas baked as "Goal achieved".viewer.html— render it in a "Result" panel (white-space: pre-wrap, HTML-escaped).Testing
pnpm lint,pnpm check:boundaries,pnpm testall green — 1921 tests (+3 newnormalizeReportregression tests covering: evidence fallback when the message is empty, message preferred when present,undefinedwhen both empty).bad viewagainst a real run: the answer now appears in the Result panel (previously hidden).