feat(query): --format sarif | annotations (B.8 — pipe rows into GitHub Code Scanning + PR annotations)#43
Merged
SutuSebastian merged 7 commits intomainfrom May 2, 2026
Merged
Conversation
…er 1 of 6) Adds the parser slice + tests for the new --format flag. No formatter wired yet — sarif/annotations values parse and propagate but render via the existing text/json paths until Tracers 2 + 3 land the actual formatters. Per docs/plans/sarif-formatter.md § D9, --format overrides --json when both passed; --json alone implies --format json (back-compat); default = text. Plan doc committed alongside (created on commit, deleted on ship per docs-governance Rule 3).
…f 6) - New application/output-formatters.ts: pure transport-agnostic formatter; SARIF 2.1.0 doc with auto-detected location columns (file_path / path / to_path / from_path priority) and optional region.startLine + .endLine. Recipes without locations emit results: [] + stderr warning (per plan § D6 + § D8). - Wired into runQueryCmd: --format sarif short-circuits to printFormattedQuery before printQueryResult; recipe description / body pulled from getQueryRecipeCatalogEntry to populate rule.shortDescription / fullDescription. - Parser combo guard: --format sarif|annotations cannot be combined with --summary / --group-by / --save-baseline / --baseline (different output shapes — sarif/annotations require flat rows). Tested. - 22 unit tests on the formatter cover location detection, message construction, region emission, ad-hoc rule id (codemap.adhoc), recipe-body fullDescription. - Annotations branch is a stub that prints "not yet implemented" + returns 1 — Tracer 3 lands the actual formatter. Verified end-to-end: bun src/index.ts query --recipe deprecated-symbols --format sarif emits a valid SARIF 2.1.0 doc with one result for the @deprecated fixture symbol.
GitHub Actions ::notice file=…,line=…::msg per row. One line per locatable row; rows without a location column are silently skipped (caller decides whether to print a stderr warning); empty input → empty string. Newlines in the message are collapsed to single spaces because the GH parser stops at the first newline. Default level 'notice'; 'warning' / 'error' overrides supported for future per-recipe severity (sarifLevel frontmatter, deferred to v1.x). Verified end-to-end: bun src/index.ts query --recipe deprecated-symbols --format annotations emits the expected ::notice line for the @deprecated fixture symbol.
…acer 5 of 6) Adds the same --format CLI surface to the MCP query / query_recipe tools so agents can request a formatted text payload directly without piping through codemap query. - New formatEnum on the inputSchema (json | sarif | annotations); 'text' is omitted because terminal-table output is useless to an agent. - formatToolIncompatibility mirrors the CLI parser's incompatibility check (sarif/annotations + summary/group_by → error). - formattedQueryResult shared helper between query and query_recipe — query_recipe passes recipeId so the SARIF rule.id derives to codemap.<recipe>; query (ad-hoc) leaves it undefined → codemap.adhoc. - 4 new MCP server tests cover SARIF on a recipe, annotations on a recipe, sarif+summary rejection, and sarif on ad-hoc SQL. query_batch deliberately not wired — annotation/sarif on a heterogeneous batch is awkward (every statement could ask for a different format) and no real consumer has asked. Defer to v1.x.
…te plan + changeset (Tracer 6 of 6)
- README.md "Daily commands" stripe: add --format <text|json|sarif|annotations> example pair (sarif > findings.sarif and annotations).
- docs/glossary.md: new 'SARIF' and 'GH annotations' entries (per Rule 9 — new domain nouns).
- docs/architecture.md: new 'Output formatters' wiring paragraph above 'Validate wiring'; covers location auto-detection, rule-id taxonomy, MCP integration, deferred-to-v1.x overrides.
- .agents/rules/codemap.md + templates/agents/rules/codemap.md (Rule 10): new 'SARIF / GH annotations' row in the CLI table.
- .agents/skills/codemap/SKILL.md + templates/agents/skills/codemap/SKILL.md: query / query_recipe tool descriptions extended with format arg semantics; query_batch deferral noted.
- .changeset/sarif-formatter.md: minor changeset (new flag).
- docs/plans/sarif-formatter.md: deleted on ship per docs-governance Rule 3.
- src/{cli/cmd-query.ts, application/output-formatters.ts}: replaced dangling cross-refs to the deleted plan with cross-refs to architecture.md § Output formatters.
🦋 Changeset detectedLatest commit: 4cd91ba The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds support for outputting ChangesSARIF / Annotations Output Formatting
Sequence DiagramsequenceDiagram
participant CLI as CLI Parser
participant Validator as Format Validator
participant Query as Query Executor
participant Formatter as Output Formatter
participant Output as stdout/stderr
CLI->>Validator: parseQueryRest(args)<br/>extract --format
Validator->>Validator: validate format vs<br/>--summary/--group-by
alt Format Incompatibility
Validator->>Output: return error
else Valid
Validator->>Query: runQueryCmd({ format })
Query->>Query: executeQuery(sql)
Query->>Formatter: detectLocationColumn(rows)
alt No Locatable Rows
Formatter->>Output: warn to stderr
Formatter->>Output: results: [] (empty SARIF)
else Locatable Rows Found
Formatter->>Formatter: format === "sarif"?<br/>buildMessageText(),<br/>construct SARIF results
Formatter->>Formatter: format === "annotations"?<br/>emit ::notice lines
Formatter->>Output: print formatted text
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 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. Review rate limit: 0/1 reviews remaining, refill in 36 minutes and 8 seconds.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: 4
🧹 Nitpick comments (1)
src/cli/cmd-query.test.ts (1)
542-602: ⚡ Quick winAdd explicit parser tests for
--formatincompatibility combos.This suite validates parsing/value errors well, but it doesn’t lock the guard behavior for
--format sarif|annotationscombined with--summary,--group-by, and baseline flags. Adding those assertions here would prevent regressions on the most failure-prone matrix.Proposed test additions
describe("parseQueryRest — --format flag", () => { + it("rejects --format sarif with --summary", () => { + const r = parseQueryRest([ + "query", + "--format", + "sarif", + "--summary", + "SELECT 1", + ]); + expect(r.kind).toBe("error"); + }); + + it("rejects --format annotations with --group-by", () => { + const r = parseQueryRest([ + "query", + "--format", + "annotations", + "--group-by", + "directory", + "SELECT file_path FROM symbols", + ]); + expect(r.kind).toBe("error"); + }); + + it("rejects --format sarif with --baseline", () => { + const r = parseQueryRest([ + "query", + "--format", + "sarif", + "--baseline=base", + "SELECT 1", + ]); + expect(r.kind).toBe("error"); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/cmd-query.test.ts` around lines 542 - 602, Add explicit tests to parseQueryRest that assert --format values "sarif" and "annotations" are rejected when used with incompatible flags: --summary, --group-by, and any baseline flags (e.g., --baseline, --baseline-compare); for each combination call parseQueryRest([...]) and assert r.kind === "error" and that r.message mentions both the offending --format value and the conflicting flag so failures are clear; add these tests alongside the existing "--format flag" describe block referencing parseQueryRest and the exact flag names to lock guard behavior and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/glossary.md`:
- Around line 383-386: Move the "### GH annotations" entry into the alphabetical
"G" section of the glossary (it currently resides under the "S" section); locate
the header "### GH annotations" and cut/paste its entire paragraph so it appears
under the existing "## G" heading (ensuring surrounding entries remain
alphabetically ordered), and verify the surrounding headings and anchor links or
cross-references still render correctly after the move.
In `@src/application/output-formatters.ts`:
- Around line 200-205: The current annotation assembly emits raw values (file,
lineN, and message) into the GitHub Actions command via lines.push(`::${level}
${params.join(",")}::${message}`), which breaks if file contains "," or ":" or
message contains "%" or CR/LF; update the code around params, file, lineN,
buildMessageText, and the final lines.push so that: percent-encode
workflow-command field values per GitHub rules (replace "%" -> "%25", "\r" ->
"%0D", "\n" -> "%0A") for the message and also escape "," -> "%2C" and ":" ->
"%3A" in parameter values (file and any param strings) before joining and
printing; ensure buildMessageText(row) is still trimmed/collapsed before
applying the encoding so annotations are well-formed.
In `@src/cli/cmd-query.ts`:
- Around line 625-631: The non-SARIF output branch still checks opts.json
instead of the resolved format, so the new --format flag is ignored; update the
result-rendering logic to use the resolved OutputFormat (e.g. a local variable
like resolvedFormat or the field opts.format) instead of opts.json when deciding
between text vs JSON rendering (and likewise ensure any checks that disallow
group-by/summary/baseline for "sarif" or "annotations" use the same resolved
format). Locate the code that reads opts.json and opts.format (and the type
OutputFormat / format?: OutputFormat) and replace conditional checks on
opts.json with checks on the resolved format variable so --format overrides
--json/--text consistently (also apply the same change to the other branch
ranges noted around the other block).
- Around line 47-60: Update the CLI help and parser usage strings to document
the new --format flag and its accepted values: reference OUTPUT_FORMATS
(text|json|sarif|annotations) and the incompatibility rule that --format
overrides --json when both are supplied (with --json remaining an alias for
--format json); specifically edit printQueryCmdHelp() and any usage/description
strings returned by the parser in this file to mention --format and the
behavior, and validate/help text should call out the default (text) and that
--json is equivalent to --format json so users see both options and the
precedence rule.
---
Nitpick comments:
In `@src/cli/cmd-query.test.ts`:
- Around line 542-602: Add explicit tests to parseQueryRest that assert --format
values "sarif" and "annotations" are rejected when used with incompatible flags:
--summary, --group-by, and any baseline flags (e.g., --baseline,
--baseline-compare); for each combination call parseQueryRest([...]) and assert
r.kind === "error" and that r.message mentions both the offending --format value
and the conflicting flag so failures are clear; add these tests alongside the
existing "--format flag" describe block referencing parseQueryRest and the exact
flag names to lock guard behavior and prevent regressions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e82d91bd-684b-45f8-a154-387a9b77fda3
📒 Files selected for processing (15)
.agents/rules/codemap.md.agents/skills/codemap/SKILL.md.changeset/sarif-formatter.mdREADME.mddocs/architecture.mddocs/glossary.mdsrc/application/mcp-server.test.tssrc/application/mcp-server.tssrc/application/output-formatters.test.tssrc/application/output-formatters.tssrc/cli/cmd-query.test.tssrc/cli/cmd-query.tssrc/cli/main.tstemplates/agents/rules/codemap.mdtemplates/agents/skills/codemap/SKILL.md
…c --format in CLI help (CodeRabbit on #43) Four CodeRabbit threads, all verified ✅ correct: 1. **(major) Text/JSON path was ignoring --format.** runQueryCmd resolved opts.format in the parser then passed opts.json (raw) to printQueryResult, breaking my own design D9: --format json (no --json) printed a terminal table; --json --format text printed JSON. Fixed by computing effectiveFormat once at the top of runQueryCmd and threading the resulting isJson boolean through every branch (saveBaseline / baselineDiff / groupedQuery / printQueryResult / emitErrorMaybeJson). Two new parser tests lock the precedence (--format text wins over --json; --format json with no flag still emits JSON). 2. **(major) formatAnnotations emitted unescaped fields.** Per actions/toolkit (https://github.com/actions/toolkit/blob/master/packages/core/src/command.ts), property values must escape % \r \n : , and message payloads must escape % \r \n; otherwise file paths with : (Windows drive letters) or , break the annotation, and messages with % get parsed as malformed escape sequences. Added escapeAnnotationData + escapeAnnotationProperty helpers (exported + 5 unit tests covering order-of-operations, empty strings, idempotence). Whitespace collapse still runs first so messages stay single-line by GH spec; the CR/LF escape paths are exercised by property values. 3. **(minor) printQueryCmdHelp didn't advertise --format.** Added the full enum + precedence + sarif/annotations incompatibility note. Both "missing SQL or recipe" usage strings updated to mention --format <fmt>. 4. **(minor) docs/glossary.md alphabetical order.** GH annotations was under ## S (next to SARIF since they shipped together); moved to a new ## G section between F and H per glossary's per-letter structure. Cross-references unchanged. Smoke verified post-fix: `codemap query --format json` emits JSON; `--json --format text` emits text. 37 new + updated unit tests pass; bun run check green.
…odeRabbit nitpick on #43) 7 new parser tests covering every incompatible combo CodeRabbit flagged: --format sarif|annotations × --summary | --group-by | --baseline | --save-baseline (both =name and default-name forms) on ad-hoc SQL + on recipes. Plus 2 negative tests confirming --format text/json compose freely with summary/group-by (text/json don't trip the guard). The guard logic existed since Tracer 2 but only had end-to-end coverage via the SARIF/annotations smoke + the MCP-side mirror tests; these direct parse-time tests prevent regressions when the parser grows new flags.
4 tasks
2 tasks
SutuSebastian
added a commit
that referenced
this pull request
May 2, 2026
…llow + competitive-scan (#45) - fallow.md row B.8 was ❌ Open; PR #43 shipped it. Updated to ✅ with implementation notes (rule.id taxonomy, location auto-detection, deferred frontmatter overrides). - fallow.md MCP-server bullet said 'HTTP API stays in roadmap backlog'; PR #44 shipped it. Reworded; added a dedicated PR #44 bullet covering the new transport, shared tool/resource handlers, CSRF/DNS-rebinding guard, Zod validation at the HTTP boundary, and 404/500 status semantics. - competitive-scan-2026-04.md '4. What moved to the roadmap': replaced 'HTTP API (codemap serve) — still backlog' with shipped rows for PR #43 + PR #44. - Fixed broken anchor: cross-ref to fallow.md was #status-snapshot-as-of-2026-05-01 but the heading bumped to 2026-05-02 in PR #42.
3 tasks
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
Adds
codemap query --format <text|json|sarif|annotations>so any recipe row-set can be piped into:::notice file=…,line=…::msgper row so PR diffs surface findings inline.Pure output-formatter additions on top of the existing JSON pipeline; no schema impact. Closes B.8 from
docs/research/fallow.md.Tracers (one commit each)
--formatflag parser (cmd-query.ts) — enum + tests + parse-time rejection of incompatible combos (--summary/--group-by/ baseline).formatSarif(application/output-formatters.ts) — pure transport-agnostic; wired end-to-end fordeprecated-symbolsrecipe; 22 unit tests on location detection, message construction, region emission, ad-hoc rule id, recipe-body fullDescription.formatAnnotations—::notice file=…,line=…::msgper row; collapses newlines (GH parser stops at first one); level override supported.index-summary,markers-by-kind) emit valid SARIF withresults: []+ stderr warning; ad-hoc SQL getsrule.id = codemap.adhoc.formatargument onquery/query_recipetools; 4 new MCP server tests; same incompatibility guard mirrored at the tool layer.query_batchdeferred to v1.x (annotation/sarif on a heterogeneous batch is awkward)..agents/+templates/agents/per Rule 10), changeset (minor); plan doc deleted on ship per Rule 3.Decisions (from the now-deleted
docs/plans/sarif-formatter.md)file_path/path/to_path/from_pathpriority + optionalline_start/line_endresults: []+ stderr warningrule.idtaxonomycodemap.<recipe-id>for--recipe;codemap.adhocfor ad-hoc SQLresult.level"note"(per-recipe override deferred to v1.x via frontmatter)--formatoverrides--json;--jsonstays as alias for--format json; defaulttextsarif/annotationsreject--summary/--group-by/ baseline at parse time (different output shapes)Behavior change
Additive — no existing flag semantics change. New
--formatflag; newformatarg on two MCP tools. Plan doc deleted on ship.Test plan
bun src/index.ts query --recipe deprecated-symbols --format sarifemits a valid SARIF 2.1.0 doc;--format annotationsemits one::noticeline per row.bun run check(format / lint / typecheck / 458+ tests / golden queries / barrel-files) green on every tracer commit.Summary by CodeRabbit
Release Notes
New Features
--formatflag tocodemap querywith support forsarif(GitHub Code Scanning),annotations(GitHub Actions inline notices),json, andtextformats.Documentation