feat(findings): render RCA readably and surface detector verdicts in findings get#59
Open
dark-sorceror wants to merge 5 commits into
Open
feat(findings): render RCA readably and surface detector verdicts in findings get#59dark-sorceror wants to merge 5 commits into
dark-sorceror wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #52
findings getis the payoff command of the detector feature, but its human output undersold the result three ways: the RCA body printed as one raw unwrapped blob with literal##/**markdown markers, the "Category" line showed only the detector template name instead of the detector's actual classification, and the command dead-ended after the RCA with no link back to the trace or hint at what to run next. This PR fixes all three, keeping--jsonbyte-for-byte unchanged.How it works
wrapMarkdownis a pure string transform (no I/O, no width detection) so it's independently unit-testable;renderFindingpassesprocess.stdout.columns ?? 80andstyler.bold. The trace-URL fetch only happens in human mode —--jsonreturns the bareFindingDetailwith no extra API call, same as before.Before:
After:
What's included
Rendering
src/render/wrap.ts(new) —wrapMarkdown(text, width, bold?): width-aware wrapping with minimal markdown treatment (heading/**bold**/`code`markers stripped or styled, never printed literally; list items get a hanging indent so continuations align under the item text, not the marker).Findings get
src/commands/findings/get.tsrenderFindingnow wrapsfinding.rca.resultthroughwrapMarkdowninstead of splitting on\nand printing verbatim.dataCategory/detectorCategoryhelpers: preferresult.data.category(loosely typed, only trusted when a non-empty string) annotated with the raw template id, e.g.Tool call error (template: failure); fall back to the existing template→label map whendatahas no usable category.Identified:line per result, surfacingresult.identified.runGetdoes a best-effortclient.getTrace(finding.trace_id)(human mode only) to print aView in TraceRoot:link from the backend-providedtrace_url; any failure degrades silently to no link. The output always ends withtraceroot traces get <trace-id>/traceroot traces export <trace-id>hints, closing the loop thattraces get's finding block opens.Tests
tests/render/wrap.test.ts(new) — 12 unit tests covering width wrapping, paragraph joining, blank-line paragraph separation, heading/bold/code marker handling with and without a styler, list hanging indent, and the long-single-word edge case.tests/commands/findings-get.test.ts— extended with cases for thedata.categoryfallback matrix (valid string, wrong type, empty string, null, non-object, array), theIdentified:line, long-paragraph wrapping, literal-markdown stripping, the trace-link footer (present and omitted-on-error), and a--jsonassertion that no extragetTracecall is made.Test plan
npm test— 611/611 vitest tests pass, including 12 newwrap.tsunit tests and the extendedfindings-getsuite (data-category fallback matrix,Identified:line, wrap/markdown-stripping cases, footer link present/omitted,--jsonno-extra-getTraceassertion)##/**in the output, RCA wrapped at 80 columns,Identified: yesandCategory: Tool call error (template: failure)rendered correctly, footer shows the backend-providedView in TraceRootURL plus bothtraces get/traces exporthints--jsonbyte-shape unchanged and issues no extra API call (verified nogetTracefetch happens in JSON mode)Known minor limitations (non-blocking, within the "minimal markdown" scope this issue asked for): an unclosed
**passes through literally rather than being auto-closed, and fenced code blocks get reflowed like ordinary paragraphs rather than preserved verbatim.Summary by cubic
Improves
findings gethuman output by wrapping and lightly styling RCA text, surfacing detector verdicts, and adding a trace link with next-step hints. Fixes #52 while keeping--jsonoutput byte-for-byte unchanged and with no extra API calls.New Features
wrapMarkdown; strip heading/bold/code markers and apply bold where supported.Identified: yes/noand improvedCategory:per result: preferresult.data.category(adds(template: <id>)), else fall back to the template label.traceroot traces get <id>andtraceroot traces export <id>hints; trace URL is fetched only in human mode and failures degrade silently.Bug Fixes
**Note**:renders asNote:).Written for commit 052478e. Summary will update on new commits.