Add JSON-for-UI output mode with published JSON Schema#2
Merged
Conversation
…rorJSON Merges plan Tasks 2 and 3: the shared sampleReport test fixture changes shape, so the BuildDocument transform and the report_test.go rewrite must land together to keep the build green.
A raw-JSON config argument that does not start with '{' (e.g. a JSON array or a
BOM-prefixed object) is misread as a file path; config.Load's os.ReadFile error
then echoes the raw input verbatim, including an inline apiKey. The --json path
emitted this to stdout (text mode to stderr). Redact using a key sniffed from
the argument itself, since the parsed cfg is empty when Load fails this way.
- Counts.add takes typed validator.Status (compile-time enum coupling) and panics on an out-of-range value instead of silently dropping it; Summary.Total is now len(rep.Results) so it can't diverge from the source. - WriteJSON/WriteErrorJSON marshal fully then write once, so a mid-stream write failure can't leave partial JSON on the consumer's stream. - Redact result messages at the report layer (defense in depth; checks already redact upstream); redact the run-error text path for consistency. - Cover the dark branches and contracts: SKIP counting, leftover/unknown-source grouping, details passthrough/omit, zero-data-sources ([] not null), determinism, message redaction, and the unknown-status panic. - Comment/spec accuracy fixes.
|
Warning Review limit reached
More reviews will be available in 40 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements a comprehensive refactoring of the ChangesJSON-for-UI Output Mode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/superpowers/plans/2026-05-25-json-ui-output-mode.md (1)
82-83: ⚡ Quick winPrefer
maketargets in run instructions for repo consistency.These steps currently instruct raw Go commands (
go test,go build,go vet,go mod tidy, etc.). Please switch to repo-standardmaketargets where equivalents exist (make test,make build,make vet,make tidy) so contributors follow one consistent workflow.Based on learnings: "Use
maketargets for common tasks (build, test, test-live, vet, fmt, run, tidy, install, clean) instead of running raw Go commands directly".Also applies to: 200-201, 333-334, 437-438, 540-541, 616-617, 693-694, 731-732, 961-962, 1023-1024
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-05-25-json-ui-output-mode.md` around lines 82 - 83, Update the documentation lines that run raw Go commands (e.g., the line showing "Run: `go test ./report/ -run 'TestSplitCheck|TestRedactString' -v`") to use the repository's make targets instead; replace `go test` with the equivalent `make test` (and similarly swap `go build`, `go vet`, `go mod tidy`, etc. for `make build`, `make vet`, `make tidy`) and adjust any test-specific flags to be passed via the make target convention used in this repo; apply the same change to the other occurrences called out (around the lines that currently show raw `go` commands at the noted sections).cmd/oba-validator/main.go (1)
89-104: 💤 Low valueConsider extracting the duplicated
"output error:"literal to a constant.The string
"output error:"appears three times (lines 94, 111, 130). While functional, extracting it to a package-level constant improves maintainability.♻️ Suggested refactor
+const outputErrPrefix = "output error:" + func run(args []string, stdout, stderr io.Writer) int { // ... if werr := report.WriteErrorJSON(stdout, err.Error(), key); werr != nil { - fmt.Fprintln(stderr, "output error:", werr) + fmt.Fprintln(stderr, outputErrPrefix, werr) } // ... (apply similarly to other occurrences)Also applies to: 107-121, 129-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/oba-validator/main.go` around lines 89 - 104, The code duplicates the literal "output error:" in main.go; introduce a package-level constant (e.g., outputErrorPrefix) and replace all literal occurrences used with fmt.Fprintln and any similar log writes (locations around the cfg loading error block and the other error-handling blocks that call fmt.Fprintln with stderr) to use that constant instead; update references in the cfg load error handling (where cfg, err, redactionKey, o.jsonOut, report.WriteErrorJSON, stdout, stderr are used) and the other error branches (lines noted in the review) so all three places use the single constant for maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-05-25-json-ui-output-design.md`:
- Around line 52-58: The fenced code block containing the flow diagram (starting
with "config.Load ──err──▶ (--json? WriteErrorJSON : text) ; exit 2" and ending
with "Report ──▶ (--json? WriteJSON=BuildDocument : WriteText) ; exit
rep.ExitCode()") needs a language tag to satisfy MD040; replace the opening
triple backticks with a tagged fence (e.g., change "```" to "```text") so the
block is fenced as text while leaving the block content unchanged.
In `@README.md`:
- Line 54: Update the malformed JSON example `{ "schemaVersion", "error" }` in
README.md to a valid JSON object: replace it with a proper key/value example
such as `{ "schemaVersion": "1.0", "error": "description" }` (or similar
concrete values) so the emitted error-object example is valid JSON; locate the
example text and perform the replacement in the documentation.
---
Nitpick comments:
In `@cmd/oba-validator/main.go`:
- Around line 89-104: The code duplicates the literal "output error:" in
main.go; introduce a package-level constant (e.g., outputErrorPrefix) and
replace all literal occurrences used with fmt.Fprintln and any similar log
writes (locations around the cfg loading error block and the other
error-handling blocks that call fmt.Fprintln with stderr) to use that constant
instead; update references in the cfg load error handling (where cfg, err,
redactionKey, o.jsonOut, report.WriteErrorJSON, stdout, stderr are used) and the
other error branches (lines noted in the review) so all three places use the
single constant for maintainability.
In `@docs/superpowers/plans/2026-05-25-json-ui-output-mode.md`:
- Around line 82-83: Update the documentation lines that run raw Go commands
(e.g., the line showing "Run: `go test ./report/ -run
'TestSplitCheck|TestRedactString' -v`") to use the repository's make targets
instead; replace `go test` with the equivalent `make test` (and similarly swap
`go build`, `go vet`, `go mod tidy`, etc. for `make build`, `make vet`, `make
tidy`) and adjust any test-specific flags to be passed via the make target
convention used in this repo; apply the same change to the other occurrences
called out (around the lines that currently show raw `go` commands at the noted
sections).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 11352a98-471f-4077-92ba-6e0007bb5225
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
CLAUDE.mdREADME.mdcmd/oba-validator/main.gocmd/oba-validator/main_test.godocs/superpowers/plans/2026-05-25-json-ui-output-mode.mddocs/superpowers/specs/2026-05-25-json-ui-output-design.mdgo.modreport/document.goreport/document_test.goreport/report.goreport/report_test.goreport/schema_test.goschema/oba-validator-report.schema.json
Address CodeRabbit review nits on PR #2: the README error-object example was not valid JSON syntax, and the spec's flow-diagram fence lacked a language tag (MD040).
|
This was referenced May 26, 2026
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
1 participant
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
--jsonnow emits a single document withmeta(run inputs — never the apiKey),summary(verdict + status counts), andgroups(aservergroup plus one per data source, each with its results andcategory/step-split check names). This replaces the previous flatReport.ResultsJSON.schema/oba-validator-report.schema.jsondescribing both the report and error variants (oneOf), with a conformance test that validates generated output against it (so the schema can't rot).api-key-serviceRender one-off-job convention: the job prints the document to stdout and the caller reads it; on failure before a report exists, a{ "schemaVersion", "error" }object is emitted to stdout and the process exits 2.Design + plan:
docs/superpowers/specs/2026-05-25-json-ui-output-design.md,docs/superpowers/plans/2026-05-25-json-ui-output-mode.md.Test plan
go build ./... && go vet ./... && go test ./...— all greenBuildDocumentgrouping/ordering, category/step split, counts/verdict/exitCode, meta echo +omitempty, apiKey never present (+ URL & message redaction), SKIP counting, leftover/unknown-source grouping, zero-data-sources ([]notnull), determinism, unknown-status panic assertion--jsonoutput shape; config-error → JSON error doc + exit 2; inline-apiKey redaction in both JSON and text error pathstotal == sum(counts), no apiKey leak; and the config-error edge caseReview notes (multi-agent review; addressed)
{(e.g. a JSON array / BOM) is misread as a file path, andconfig.Load's read error echoed the raw input — including an inline apiKey — to stdout under--json. Now redacted via a key sniffed from the argument.Counts.addtakes the typedvalidator.Status(panics on out-of-range instead of silently dropping);Summary.Total=len(results);WriteJSONmarshals fully then writes once (no partial JSON on a broken pipe); result messages redacted at the report layer as defense-in-depth.Non-blocking items surfaced for reviewer judgment
verdict: "PASS"/exitCode: 0(per the existing severity model — skips never affect exit code). A UI may want to distinguish "passed" from "nothing actually validated" usingcounts.skip.Detailspass through un-redacted by design (checks redact at source). If a future check puts a URL/secret inDetails, the report layer won't catch it — could add deep redaction later.meta. OBA feed URLs are public in practice; flagging since it's a new disclosure surface.Summary by CodeRabbit
New Features
--json) now provides structured data including metadata, summary statistics, and grouped results for better machine readabilityDocumentation