feat: add altimate-code check CLI command for deterministic SQL checks#453
Merged
anandgupta42 merged 7 commits intomainfrom Mar 25, 2026
Merged
feat: add altimate-code check CLI command for deterministic SQL checks#453anandgupta42 merged 7 commits intomainfrom
altimate-code check CLI command for deterministic SQL checks#453anandgupta42 merged 7 commits intomainfrom
Conversation
Add a new `check` subcommand that runs deterministic SQL analysis (lint, validate, safety, policy, PII, semantic, grade) without requiring an LLM. The command: - Accepts SQL files as positional args or globs (defaults to `**/*.sql`) - Calls `Dispatcher.call()` for each check category (no direct napi imports) - Processes files in batches of 10 for concurrency - Supports `--format json|text`, `--severity`, `--fail-on` flags - Outputs diagnostics to stderr, structured JSON to stdout - Does NOT call `bootstrap()` — no session/db/server needed - Registered in `index.ts` with proper `altimate_change` markers
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review.
Tip: disable this comment in your organization's Code Review settings.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a new deterministic CLI command Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Cmd as Check Command
participant FileIO as File I/O
participant Dispatcher as Dispatcher Backends
participant Normalizer as Result Normalizer
participant Formatter as Output Formatter
participant Proc as Process/Exit
User->>Cmd: invoke `altimate-code check` with options
Cmd->>FileIO: discover/expand globs and read files
FileIO-->>Cmd: list of SQL files / file contents
Cmd->>FileIO: read schema & policy files (if provided)
FileIO-->>Cmd: schema/policy contents
Cmd->>Dispatcher: batch calls per-file per-check (lint, validate, safety, policy, pii, semantic, grade)
Dispatcher-->>Cmd: raw backend responses
Cmd->>Normalizer: map responses -> `Finding[]` (severity, message, location, code, rule, suggestion)
Normalizer-->>Cmd: normalized findings
Cmd->>Cmd: filter by `--severity`, aggregate per-category, compute pass/fail (`--fail-on`)
Cmd->>Formatter: format as JSON or human-readable text
Formatter-->>Cmd: formatted report
Cmd->>User: print report and duration
alt pass
Cmd->>Proc: exit 0
else fail
Cmd->>Proc: set exitCode 1 (allowing cleanup)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Merged
6 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/cli/cmd/check.ts`:
- Around line 529-545: The CI gating (pass/fail) is being computed from the
filtered `results` so flags like `--severity error --fail-on warning` can bypass
failure; instead compute counts from the raw unfiltered findings (`allResults`)
before applying `filterBySeverity`. Update the logic around
`allFindings`/`errors`/`warnings` to derive these counts from
Object.values(allResults).flatMap(...) (use the original raw findings) and keep
the display `results` = toCategoryResult(filterBySeverity(...)) as-is; ensure
`failOn`/`pass` decisions use the raw counts and not the filtered `results`.
- Around line 91-99: The fallback severity branches are dead because
normalizeSeverity() always returns a value; change each severity expression to
only call normalizeSeverity when the upstream severity is present and otherwise
use the intended default. For example, replace expressions like
normalizeSeverity(f.severity) || ("error" as const) with a guard such as
(f.severity != null ? normalizeSeverity(f.severity) : ("error" as const)), and
apply the same pattern for the other locations that currently use || ("info" as
const) or similar so missing severities yield the correct default instead of
being mapped to warning.
- Around line 253-272: runGrade currently only maps recommendation-like findings
and drops the actual grade returned by altimate_core.grade; update runGrade to
extract the grade/score from result.data (e.g., result.data.grade or
result.data.score) and attach it into the CheckCategoryResult metadata so
downstream formatting can print it. Specifically, in runGrade (and the similar
mappings at the other spots noted) return or include a metadata object on the
category result containing the raw grade value (e.g., metadata: { grade: <value>
}) and ensure the code paths that build results.grade/CheckCategoryResult
preserve and pass that metadata through to the formatter that prints --checks
grade. Locate the mapping logic in runGrade and the other grade-handling blocks
and add the grade extraction and metadata insertion while leaving existing
finding mapping intact.
- Around line 413-416: Replace direct process.exit() invocations in the check
command handler with setting process.exitCode and returning so the outer finally
block can run; specifically, in the validation branch checking checks.length
(currently calling process.exit()), set process.exitCode = 1 and return; do the
same for the other validation/error branches (the pre-check validation errors
and the "policy file read error" branch) by assigning process.exitCode = 1 then
returning; for the "no SQL files found" branch return without changing exitCode;
and for the final "checks failed" branch set process.exitCode = 1 and return
instead of calling process.exit(). Ensure all occurrences of process.exit() in
the check command handler are replaced accordingly so Telemetry.shutdown() in
the outer finally will execute.
In `@packages/opencode/src/index.ts`:
- Around line 206-207: CheckCommand is being registered through the global
middleware path which triggers JsonMigration.run() and full stateful
initialization; move CheckCommand to a fast path that bypasses the middleware
and migration so deterministic SQL checks only use Dispatcher.call() without
starting JsonMigration or other stateful init. Concretely: register or handle
CheckCommand before applying the middleware chain (the code that runs lines
96-175 and calls JsonMigration.run()), or add a short-circuit in the command
dispatcher that detects the CheckCommand name and directly invokes
Dispatcher.call() for deterministic SQL checks, ensuring no code paths call
JsonMigration.run() or any stateful initialization when handling CheckCommand.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7dfcf22d-e218-4df6-b07a-3f237f1933db
📒 Files selected for processing (2)
packages/opencode/src/cli/cmd/check.tspackages/opencode/src/index.ts
Comment on lines
+91
to
+99
| return errors.map((f) => ({ | ||
| file, | ||
| line: f.line as number | undefined, | ||
| column: f.column as number | undefined, | ||
| code: f.code as string | undefined, | ||
| rule: "validate", | ||
| severity: normalizeSeverity(f.severity as string) || ("error" as const), | ||
| message: (f.message ?? f.description ?? "") as string, | ||
| suggestion: f.suggestion as string | undefined, |
There was a problem hiding this comment.
The per-check fallback severities never fire.
normalizeSeverity() always returns a value, so the || "error" / || "info" branches on Lines 97, 169, and 269 are dead code. Missing severities are therefore classified as warning, which can let validate/policy failures slip past --fail-on error and upgrades grade recommendations from info to warning.
Suggested fix
- severity: normalizeSeverity(f.severity as string) || ("error" as const),
+ severity: f.severity ? normalizeSeverity(f.severity as string) : "error",
...
- severity: normalizeSeverity(f.severity as string) || ("error" as const),
+ severity: f.severity ? normalizeSeverity(f.severity as string) : "error",
...
- severity: normalizeSeverity(f.severity as string) || ("info" as const),
+ severity: f.severity ? normalizeSeverity(f.severity as string) : "info",Also applies to: 163-170, 263-270, 283-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/cli/cmd/check.ts` around lines 91 - 99, The fallback
severity branches are dead because normalizeSeverity() always returns a value;
change each severity expression to only call normalizeSeverity when the upstream
severity is present and otherwise use the intended default. For example, replace
expressions like normalizeSeverity(f.severity) || ("error" as const) with a
guard such as (f.severity != null ? normalizeSeverity(f.severity) : ("error" as
const)), and apply the same pattern for the other locations that currently use
|| ("info" as const) or similar so missing severities yield the correct default
instead of being mapped to warning.
…eck` command - Add `docs/docs/usage/check.md` with full documentation covering all 7 check types, CLI options, JSON output schema, policy file format, schema file format, severity levels, CI/CD integration (GitHub Actions, pre-commit, GitLab CI), and real-world usage examples - Update `docs/docs/usage/cli.md` to reference the `check` command in the subcommands table - Add `check` page to `docs/mkdocs.yml` navigation under Interfaces - Extract helper functions (`normalizeSeverity`, `filterBySeverity`, `toCategoryResult`, `formatText`, `buildCheckOutput`) into `check-helpers.ts` for testability - Refactor `check.ts` to import from `check-helpers.ts` (no behavior change) - Add 53 unit tests covering: severity normalization, severity filtering, category result building, text formatting, output assembly, constants validation, and edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/docs/usage/check.md`:
- Around line 154-163: Update the fenced code block that shows the CLI output by
adding a language specifier to the opening triple-backticks (e.g., change ``` to
```text or ```plaintext) so linters and renderers treat it as plain text and
enable proper highlighting; locate the triple-backtick block that begins with
"Checked 3 file(s) with [lint, safety]" and modify its opening fence
accordingly.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ad341f43-688d-40a6-8206-26a11105daa5
📒 Files selected for processing (6)
docs/docs/usage/check.mddocs/docs/usage/cli.mddocs/mkdocs.ymlpackages/opencode/src/cli/cmd/check-helpers.tspackages/opencode/src/cli/cmd/check.tspackages/opencode/test/cli/check.test.ts
✅ Files skipped from review due to trivial changes (2)
- docs/docs/usage/cli.md
- docs/mkdocs.yml
- 58 new tests covering full handler flow with mocked `Dispatcher.call()` - E2E: all 7 check types, `--fail-on` exit codes, `--severity` filtering, `--policy` validation, unknown check names, schema resolution, batching, text/JSON output, mixed success/failure across checks - Adversarial: null bytes, shell metacharacters, 100K-char lines, Unicode/emoji, CRLF, spaces in filenames, deeply nested paths, malformed policy JSON, 1MB policy files, undefined/null finding fields, 5000 findings, XSS-like content, non-string severity, `__proto__` pollution, binary `.sql` content, symlinks, directory with `.sql` extension, duplicate file args, empty checks string - Fix: `normalizeSeverity()` now handles non-string inputs without crashing (previously threw on numeric/boolean/object severity from Dispatcher) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. `--fail-on` now evaluates UNFILTERED findings before `--severity`
filtering. Previously, `--severity error --fail-on warning` would
false-pass because warnings were filtered out before exit logic.
2. Removed unused CLI options (`--dialect`, `--dbt-project`, `--manifest`)
that were documented but had no effect — misleading for CI users.
3. `runPii` now checks `result.success` before processing data,
consistent with all other check runners.
4. Dispatcher failures now emit error-severity findings instead of
silently returning `[]`. A broken native bridge no longer reports
PASS with zero findings.
Also:
- Use `buildCheckOutput()` helper instead of duplicated inline logic
- Remove dead `|| ("error" as const)` fallbacks after `normalizeSeverity`
- Add 4 new tests: severity/fail-on interaction, runPii failure,
Dispatcher failure exit code
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Replace `process.exit()` with `process.exitCode` + `return` so the outer `finally` block in `index.ts` can run `Telemetry.shutdown()`. 2. Preserve A-F grade value from `altimate_core.grade` response in `CheckCategoryResult` metadata (`grade`, `score` fields). 3. Skip DB migration for `check` command — it only needs `Dispatcher` and has zero database dependencies. Prevents startup failure on read-only CI environments and removes multi-minute overhead. 4. Remove unused `--dialect`, `--dbt-project`, `--manifest` from docs options table (code already removed in prior commit). 5. Add `text` language specifier to fenced code block in `check.md`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_context` `schema_context` is already optional (`?`) in the Dispatcher types. The `undefined as any` casts were unnecessary type safety bypasses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 participants
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.
What does this PR do?
Adds a new
altimate-code checkCLI command that runs deterministic SQL checks without an LLM. This is the foundation for thealtimate-code-actionsGitHub Action to leverage all altimate-core capabilities.Usage:
altimate-code check models/*.sql --format json --checks lint,safety,validate,policy,piiSupported checks (7):
lint— 26 anti-pattern rules (L001-L026) viaaltimate_core.lintvalidate— DataFusion SQL validation viaaltimate_core.validatesafety— SQL injection detection (10 rules) viaaltimate_core.safetypolicy— YAML guardrail enforcement viaaltimate_core.policypii— PII column detection viaaltimate_core.query_piisemantic— Semantic validation (cartesian, NULL, JOINs) viaaltimate_core.semanticsgrade— SQL quality grading (A-F) viaaltimate_core.gradeKey design decisions:
bootstrap()— no session, database, or server neededDispatcher.call()→ altimate-core napi-rs (proper architecture)--fail-oncontrols exit code for CI gatingType of change
How did you verify your code works?
Checklist
Summary by CodeRabbit
checkCLI command for deterministic SQL checks (lint, validate, safety, policy, PII, semantic, grade) with file auto-discovery, glob support, batching, severity filtering, JSON or text output, duration reporting, and configurable CI-friendly exit behavior.