chore: upgrade @altimateai/altimate-core to 0.7.0 and sync consumer contracts

[anandgupta42]

Issue for this PR

Closes#1089

Type of change

• Bug fix
• Refactor / code improvement

What does this PR do?

Upgrades @altimateai/altimate-core 0.5.1 → 0.7.0 and syncs every consumer to the real engine output shapes.

The 0.6.0/0.7.0 releases are correctness releases: more lineage edges (derived tables, CTE chains, CTAS/INSERT…SELECT), more PII exposures, stricter migration verdicts, a new unbalanced_quote injection rule, and validate/transpile/equivalence fixes. The only type-surface change is a new required PiiColumnAccess.query_targets: string[] (the SELECT-list aliases exposing a PII column).

Integrating the new engine surfaced consumers still reading legacy (pre-native-core) shapes, all fixed here:

• Migration tool falsely reported SAFE (worst one): it read data.risks, which the engine never returns (findings/safe/overall_risk), so even ALTER TABLE … DROP COLUMN rendered "Migration: SAFE". An empty dialect string also crashed Schema.fromDdl and still rendered SAFE. Now reads the real MigrationResult shape, coerces "" → undefined (same pattern as the equivalence handler), and never renders SAFE on an engine error.
• altimate check --checks safety ignored data.threats, collapsing real ThreatFindings into one generic warning; normalizeSeverity also degraded high/medium to info, so --fail-on warning silently passed high-risk injections. Now maps rule/message/detail and high → error, medium → warning.
• altimate check --checks pii read column_name/pii_type (emitting "PII detected: unknown") and stuffed the column name into the numeric column-position field. Now maps PiiColumnAccess, reports the exposing alias from query_targets, and stringifies { Custom: string } classifications (the query-pii tool renderer printed [object Object] for those).
• Track-lineage tool always reported "0 edges": it read flat data.edges but the engine returns queries[].edges + impact_map. Also renders ERROR instead of "0 edges" when the engine call fails.
• Two tests locked to 0.5.1 parser quirks: the dialect-forwarding fixture used payload:f, which 0.7.0's default dialect now parses, so it stopped discriminating — switched to Snowflake time-travel AT(OFFSET => -60), which still does. The grade star-vs-explicit comparison compared different queries (WHERE + different tables); made projection-only.

Why these work: every mapping was written against the installed engine's .d.ts and verified by executing the real napi binary (not mocks) in the new tests.

How did you verify your code works?

• bun run typecheck clean; bun test test/altimate test/cli: 4750 pass / 0 fail (includes ~4700 real-engine tests).
• New real-engine regression tests: query_targets contract + rendering, migration destructive/safe/empty-dialect/engine-error + tool-title, track-lineage edge surfacing + error rendering, check CLI ThreatFinding + PiiColumnAccess shapes (incl. { Custom } and high → error normalization). Previously vacuous migration e2e assertions (toBeDefined()) strengthened.
• Full suite run twice: the only failures (MCP headers/HttpApi, TUI sound decode, run-subprocess) were reproduced identically with 0.5.1 pinned or shown run-to-run flaky with zero engine coupling — pre-existing, not from this PR.
• Real-binary smoke: altimate check on a SQL file against the 0.7.0 engine; injection-breakout payload now flagged by isSafe; BigQuery equivalence decidable.
• Marker check: analyze.ts --markers --base main --strict — no upstream-shared files modified.
• Codex reviewed twice: first pass found the legacy-shape consumers (fixed here); second pass verified each fix against the installed engine and confirmed the tests are non-tautological.
• Not verified: Windows/Linux native binaries (darwin-arm64 only locally; CI covers the rest).

Screenshots / recordings

Not a UI change.

Round 2 — consensus-review fixes (00d0a02)

A 4-model consensus review of this PR found the contract sync incomplete: the same legacy-shape bug class survived in consumers this PR had not touched — including two silently-dead CI gates (check --checks validate passed every invalid file; check --checks semantic and the semantics tool hid findings behind the valid flag, which means "plannable", not "clean"), a dead grade check reading fields evaluate() never returns, schema-level PII detection reading findings where the engine returns columns (zero findings ever), a compare tool that rendered IDENTICAL for different queries, a policy tool that always rendered VIOLATIONS FOUND, four tools crashing on unknown dialect '' when dialect was omitted, and { Custom } PII classifications rendering [object Object] (including in the signed review verdict). All are latent since the Python-engine elimination — the 0.5.1↔0.7.0 type contracts are byte-identical except query_targets (verified by diffing both npm tarballs). All fixed in 00d0a02467 with shared dialectHint/classificationToString helpers, fail-closed gates, and ~25 new real-engine + CLI-shape regression tests. Codex verified each fix against the installed binary; its five review findings (parse_error abstention, grade fail-closed, [byteOffset, byteLength] location semantics, contradictory failure outputs, untracked files) are addressed. Upstream issue filed for the stale SafetyRule union: altimate-core-internal#764.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches CI gates (validate, safety, PII, semantic, grade) and PR review check composition; incorrect mapping could still false-pass or over-fail, but changes are heavily regression-tested against the real engine.

Overview
Upgrades @altimateai/altimate-core from 0.5.1 to 0.7.0 and aligns CLI, tools, review runner, and native handlers with the engine’s real JSON shapes so checks and UIs stop false-passing or mislabeling results.

Introduces engine-coerce (dialectHint, PII classification stringification, confidence bands, piiColumnsFromReport) and applies dialectHint everywhere dialect is optional so "" means auto-detect instead of crashing. altimate_core.check now wires PII via checkQueryPii, diff-scopes safety threats and PII against base_sql (validation stays full-scan), and normalizes flat schema_context for lintDiff.

Tools read the correct fields (allowed/violations/warnings, diffs/identical, findings vs valid, queries[].edges, pii_columns/query_targets) and use ERROR titles when the engine fails or abstains (parse_error), not SAFE/IDENTICAL/CLEAN. Schema PII detection uses columns from PiiReport and fails closed on scan errors.

altimate check fixes dead gates: validate on data.valid, semantic on findings not valid, safety on threats with high→error severity, PII/grade/policy mappings, per-file grades, and fail-closed envelopes. Review runner folds validation/safety into check issues, maps PII via query_targets, and uses shared coercions.

Large regression test additions lock these contracts against the live NAPI binary.

^{Reviewed by Cursor Bugbot for commit b35057f. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Upgrades @altimateai/altimate-core to 0.7.0 and realigns CLI/tools/review to the engine’s JSON contracts so checks stop false‑passing and titles/severities match results. Safety and PII are now diff‑scoped; validation stays non‑diff‑scoped to avoid hiding new breakage.

• Alias‑aware PII scoping: multiset subtraction on (table, column, sorted query_targets); recompute risk_level/risk_score when exposures are pre‑existing or partially filtered; stringify { Custom }; drop null suggested_masking; detector fails closed and reports success:false on partial column errors.
• Safety scoping: multiset subtraction on (rule, matched_pattern) against base_sql; recompute safe/risk_score; high|critical → error, medium → warning; envelope failures append an error finding even when partial threats exist.
• Contract sync: compare uses diffs/identical; policy uses allowed and surfaces advisory warnings; grade uses overall_grade and aggregates nested validation/safety/lint findings with locations/suggestions; semantics reads findings (not valid); validation gates on data.valid; lineage reads queries[].edges.
• Abstentions/errors: never render SAFE/IDENTICAL/CLEAN on engine errors; PII parse_error marks metadata.success:false and renders “check skipped”; semantics abstentions are error‑severity with a schema, warning when schema‑less.
• Dialect coercion: "" means auto‑detect via shared EngineCoerce.dialectHint across all dialect‑taking handlers; composite diff path fixes flat schema_context via SchemaResolver.normalizeSchemaContext.
• Review lane: only surfaces validation with a real schema; normalizes safety threat severities; PII issues prefer output aliases from query_targets and avoid duplicating source columns; CLI renders byte‑range locations.

^{Written for commit b35057f. Summary will update on new commits.}

Summary by CodeRabbit

• New Features

  • Enhanced migration results with risk levels, findings, mitigations, rollback SQL, and clearer safe, risky, and error outcomes.
  • Improved lineage displays with query relationships, transformations, impact maps, and clearer engine error reporting.
  • Expanded PII details with classifications, exposed query targets, masking suggestions, and improved column detection.
  • Improved safety checks with clearer threat labels, severity handling, and recommendations.
  • Improved validation, semantic, policy, comparison, and grade check results.

• Bug Fixes

  • Improved handling of dialects, qualified column names, backend response formats, parse errors, and safe informational findings.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR updates Altimate Core to 0.7.0. It normalizes dialect handling, reads new engine response fields across analysis tools, improves failure reporting, and expands regression coverage.

Changes

Engine contract synchronization

Layer / File(s)	Summary
Shared engine normalization packages/opencode/package.json, packages/opencode/src/altimate/native/*, packages/opencode/src/altimate/review/runner.ts, packages/opencode/test/altimate/*	Shared helpers normalize dialect, classification, confidence, and PII report columns. Native handlers and review extraction use the normalized values.
Tool contract updates packages/opencode/src/altimate/tools/*, packages/opencode/test/altimate/*	Tools read updated engine fields for migration, lineage, compare, policy, semantics, safety, and PII results. Error, skipped, finding, and success output now reflects engine state.
CLI validation and regressions packages/opencode/src/cli/cmd/*, packages/opencode/test/cli/check-e2e.test.ts, packages/opencode/test/altimate/altimate-core-e2e.test.ts	The CLI maps updated validation, safety, PII, semantic, and grade fields. Tests cover response shapes, failure envelopes, parse errors, dialect handling, multi-file grades, and formatting.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

• Upgrade @altimateai/altimate-core to 0.7.0 and sync consumer contracts #1089 — Covers the Altimate Core 0.7.0 upgrade and related consumer-contract changes.

Possibly related PRs

• AltimateAI/altimate-code#919 — Shares dialect normalization and PII handling in Altimate Core integration.
• AltimateAI/altimate-code#925 — Introduced an earlier @altimateai/altimate-core dependency update.
• AltimateAI/altimate-code#928 — Shares native handlers and dialect-forwarding changes.

Suggested reviewers:ralphstodomingo, sahrizvi

Poem

A rabbit checks each engine field,
While dialect hints are cleanly sealed.
PII columns join the trail,
Clear errors mark each failed tale.
New findings guide the test parade—
Contracts are safely upgraded.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the dependency upgrade and the related consumer contract synchronization.
Description check	✅ Passed	The description includes the required issue, change type, implementation details, verification results, screenshots note, and completed checklist.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/altimate-core-0.7.0

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts (1)

21-27: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Reuse the flattened edge list.

collectEdges(data) runs at Line 21 and Line 55. This traverses every edge twice and allocates two arrays. Pass the first result to formatTrackLineage to avoid duplicate work for large responses.

Proposed refactor

- const edgeCount = collectEdges(data).length+ const edges = collectEdges(data)+ const edgeCount = edges.length
...
- output: error ? `Error: ${error}` : formatTrackLineage(data),+ output: error ? `Error: ${error}` : formatTrackLineage(data, edges),
...
-function formatTrackLineage(data: Record<string, any>): string {+function formatTrackLineage(data: Record<string, any>, edges: any[]): string {
...
- const edges = collectEdges(data)

Also applies to: 53-55

🤖 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 `@packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts` around
lines 21 - 27, Reuse the flattened edge list created by collectEdges in the
surrounding track-lineage execution flow: update formatTrackLineage and its call
site to accept and use that existing list, removing the second collectEdges
invocation while preserving the current output behavior.

🤖 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 `@packages/opencode/test/cli/check-e2e.test.ts`:
- Around line 596-630: Make the dispatcher-based tests, including the safety
check around setDispatcherResponse and installDispatcherMocks, safe from
concurrent execution by serializing them or using an isolated dispatcher
instance. Ensure Dispatcher.reset() cannot clear or replace handlers used by
another test, and populate and restore savedHandlers when applying the test
setup.
---
Nitpick comments:
In `@packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts`:
- Around line 21-27: Reuse the flattened edge list created by collectEdges in
the surrounding track-lineage execution flow: update formatTrackLineage and its
call site to accept and use that existing list, removing the second collectEdges
invocation while preserving the current output behavior.

🪄 Autofix

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 Plus

Run ID: 6d28e9bc-8c65-4703-86d5-a101948b8b32

📥 Commits

Reviewing files that changed from the base of the PR and between 54a8f32 and 6b16d7f.

⛔ Files ignored due to path filters (1)

• bun.lock is excluded by !**/*.lock

📒 Files selected for processing (11)

• packages/opencode/package.json
• packages/opencode/src/altimate/native/altimate-core.ts
• packages/opencode/src/altimate/tools/altimate-core-check.ts
• packages/opencode/src/altimate/tools/altimate-core-migration.ts
• packages/opencode/src/altimate/tools/altimate-core-query-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts
• packages/opencode/src/cli/cmd/check-helpers.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts
• packages/opencode/test/altimate/altimate-core-native.test.ts
• packages/opencode/test/cli/check-e2e.test.ts

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review (90e2eae..b35057f)

Reviewed the incremental commit since the last review. The changes extend the
composite altimate_core.check diff-scoping in three correctness-focused ways,
plus a convention-alignment re-export.

• Safety risk_score recomputation: Previously, diff-scoped threats left
  risk_score at the full-head value (or 0). Now, when threats are filtered,
  risk_score is recomputed from the surviving severities via a documented
  max-severity approximation (critical:0.98, high:0.95, medium:0.6,
  low:0.3); when no threats are filtered the original engine value is kept.
  The empty-remaining → 0 and safe: true behavior is preserved. The logic
  is sound (filtering only removes, so remaining.length === threats.length
  correctly detects "unchanged").

• PII diff-scoping keys on output aliases: The exposure identity now
  includes sorted query_targets, so a PR that adds/renames a SELECT-list alias
  for an already-exposed column is correctly treated as a new output exposure.
  [...arr].sort() avoids mutating the engine-returned array; set subtraction is
  appropriate here (identical (table,column,aliases) tuples are the same
  exposure, unlike safety threats which need multiset subtraction).

• PII risk_level reset: When every exposure is pre-existing,
  risk_level is reset to "None" and accesses_pii to false, preventing a
  stale risk claim. Partial-filter keeps the original (conservative).

• SchemaResolver self-re-export:schema-resolver.ts gains the repo's
  standard export * as SchemaResolver from "./schema-resolver" (matches
  AGENTS.md module-shape convention); the one consumer updated to use it.

New real-engine tests are non-tautological: they verify the stale-risk reset,
alias-keyed diff-scoping (renamed alias surfaces, identical alias filters), and
the query_targets contract. No issues found.

Files Reviewed (3 files)

• packages/opencode/src/altimate/native/altimate-core.ts
• packages/opencode/src/altimate/native/schema-resolver.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts

Previous Review Summaries (13 snapshots, latest commit 90e2eae)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 90e2eae)

Status: No Issues Found | Recommendation: Merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	0

Incremental review (2762415..90e2eae)

Reviewed the single incremental commit since the last review. The changes
harden the composite altimate_core.check diff-scoping: safety subtraction
moved from a Set to a multiset on (rule, matched_pattern) so a PR
adding a second identical injection still reports it, and safe/risk_score
are recomputed to true/0 when every threat is pre-existing (preventing a
stale unsafe verdict). PII exposure is now diff-scoped against base_sql by
table|column. Validation is explicitly not diff-scoped (documented: the
engine reports only the first error, so base subtraction could hide new
breakage).

Other fixes: lintDiff now receives a normalized SchemaDefinition JSON
(newly-exported normalizeSchemaContext) instead of raw JSON.stringify,
fixing a "missing field tables" crash on flat agent schemas; the policy tool
surfaces warnings in telemetry findings; and runSemantic fails closed
(error) on an unplannable query when a schema is supplied, warning only when
schema-less. New non-tautological tests cover the stale-unsafe fix, PII
diff-scoping, and validation non-diff-scoping. The PiiDetector self-reexport
matches the repo's AGENTS.md module-shape convention.

No issues found.

Files Reviewed (6 files)

• packages/opencode/src/altimate/native/altimate-core.ts
• packages/opencode/src/altimate/native/schema-resolver.ts
• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/tools/altimate-core-policy.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts

Previous review (commit 2762415)

Status: No Issues Found | Recommendation: Merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	0

Incremental review (d4b1282..2762415)

Reviewed the 5-file incremental diff since the last review. The changes add
diff-scoped safety (subtract pre-existing base threats by (rule, matched_pattern),
fail-open to more findings on base-scan error), gate validation errors on a
real schema (hasTables) to avoid flooding lint-only reviews, and surface
policy advisory warnings as info (non-failing) findings. Both behaviors are
covered by new non-tautological tests (real-engine + mocked). No issues found.

Files Reviewed (5 files)

• packages/opencode/src/altimate/native/altimate-core.ts
• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts
• packages/opencode/test/cli/check-e2e.test.ts

Previous review (commit d4b1282)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (6 files)

• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/altimate/tools/altimate-core-semantics.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/pii-detector-e2e.test.ts
• packages/opencode/test/altimate/tool-error-propagation.test.ts
• packages/opencode/test/cli/check-e2e.test.ts

Previous review (commit 494407e)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
packages/opencode/src/cli/cmd/check.ts	332	Inclusive byte-range formatting (Math.max(loc[1] - 1, 0)) is duplicated verbatim here and at check.ts:119 — this commit applied the identical fix to both copies; extract a shared helper to prevent drift

Incremental review: 7 files (4358bb4 → 494407e)

Incremental pass over the post-approval follow-up commit. The correctness changes are sound:

• check.tsrunSafety: split the !result.success gate out of safe === false so a crashed engine fails closed via dispatcherErrorFinding instead of a bare warning; inclusive byte-range end (loc[0] + Math.max(loc[1] - 1, 0)) is correct and guards length-0.
• check.tsrunPolicy / runSemantic / runPii: remediation fallback, validation_errors detail surfacing, and columnName (machine-readable, flows to JSON output) are all correctly mapped.
• check.tsrunGrade: inclusive byte-range + nested findings normalization retained.
• runner.ts: validation.errors/safety.threats concatenated into rawIssues; grade prefers overall_grade. No double-count risk (distinct composite buckets).
• engine-coerce.ts: classificationToString default PII → UNKNOWN aligns with pii-detector.ts and is more honest for an unrecognized classification.
• sql/register.ts: EngineCoerce.dialectHint threaded into formatSql/columnLineage.
• pii-detector-e2e.test.ts: NAPI-absent skip guard prevents crashes; check-e2e.test.ts fixture + inclusive-range assertions are internally consistent.

Only one minor redundancy (see inline). No bugs, security, or fail-open regressions in the changed lines.

Fix these issues in Kilo Cloud

Previous review (commit 4358bb4)

Status: No Issues Found | Recommendation: Merge

Incremental review from 846cf24bc → 4358bb4d46 (round-7 follow-through). No new issues in the changed code.

• check.tsrunGrade nested-findings flattening refined. Validation errors now lift line/column out of the nested location object and derive suggestion from suggestions[0] (string or {message}), falling back to top-level fields. Safety threats encode their [byteOffset, byteLength] range into the message ( (bytes N-M)), reuse detail as the suggestion, and neutralize the array location (via location: undefined) so it isn't misread as a line number by the shared mapper. The byte math loc[0]-loc[0]+loc[1] and the Array.isArray(loc) runtime guard are consistent with the documented [offset, length] shape, and normalizeSeverity still maps high → error.
• E2E test strengthened to assert the flattened fields survive the mapper: validate carries line/column/suggestion, and tautology_attack carries the byte-range message + detail-derived suggestion.

No redundancy or simplification opportunities in the changed lines — the defensive as casts against the unknown engine result and the location: undefined override are intentional, not removable noise.

Files Reviewed (2 files)

• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/cli/check-e2e.test.ts

Previous review (commit 846cf24)

Status: No Issues Found | Recommendation: Merge

Incremental review from 3974e35c9 → 846cf24b (round-6 follow-through). No new issues in the changed code.

• runner.tspiiColumns — prefer output aliases over the source name. When a PII column exposes non-empty query_targets, only those aliases are reported; the source column name is the fallback. This avoids flagging a source column that never appears in the model output (e.g. SELECT email AS contact). Empty/null target entries are still filtered by typeof c === "string" and de-duped via Set.
• check.ts grade findings now merge all nested sections.lint.findings, validation.errors, and safety.threats are combined into the finding list, so a failing grade with clean lint no longer renders an empty (passing) list. Validation entries normalize to rule:"validate"/severity:"error"; safety threats keep their own rule (default "safety").
• New E2E test covers the clean-lint + validation/safety path (2 findings, rules validate + tautology_attack, one error severity).

No redundancy or simplification opportunities in the changed lines — the defensive casts against the unknown engine result are intentional, not removable noise.

Files Reviewed (3 files)

• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/cli/check-e2e.test.ts

Previous review (commit 3974e35)

Status: No Issues Found | Recommendation: Merge

Incremental review of commit 3974e35c9 (round-5 follow-through since 960229d4a). This commit tightens three fail-open paths surfaced in earlier review rounds and introduces no new issues:

• schema_detect_pii now fails closed. When detectPii returns success:false (any column classification threw, or the live-introspection catch path fired), the tool renders PII Scan: ERROR with an "incomplete" message and still dumps any partial findings — instead of the false-clean "no findings" verdict it produced when the failure coincided with finding_count === 0.
• altimate_core_policy surfaces warnings on a pass.allowed:true with a non-empty warnings[] now lists each warning (rule/message) under the pass banner rather than rendering a bare "passes all policy checks" that hid them.
• Review runner exposes PII output aliases.piiColumns now flattens each PiiColumnAccess.query_targets (the SELECT-list aliases exposing a source PII column) alongside the source column name, with the filter tightened to typeof c === "string" since target entries are spread directly. Dedup via Set keeps the list clean.
• check --checks grade single-file gate hardened. Flat grade/score are now set only when files.length === 1 && graded.length === 1, so a multi-file run where all-but-one file errored (leaving exactly one grade) no longer misattributes that lone grade as the overall result.

Each change is covered by new real-shape tests in tool-error-propagation.test.ts (fail-closed with zero and partial findings; policy warnings-on-pass). No redundancy or simplification opportunities found in the changed lines.

Files Reviewed (5 files)

• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/altimate/tools/altimate-core-policy.ts
• packages/opencode/src/altimate/tools/schema-detect-pii.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/tool-error-propagation.test.ts

Previous review (commit 960229d)

Status: No Issues Found | Recommendation: Merge

Incremental review of commit 960229d4a (round-4 fixes since a00fb3866). This commit resolves both findings carried forward from the prior round and introduces no new issues:

• Resolved (was WARNING): The composite altimate_core.check no longer swallows a thrown checkQueryPii into an empty pii = {}. It now sets pii = { parse_error: String(e) }, and formatCheck/formatCheckTitle render "PII check skipped" instead of the false-clean "No PII detected."
• Resolved (was SUGGESTION): The duplicated bandConfidence is gone — review/runner.ts now imports the shared EngineCoerce.bandConfidence (its local copy is deleted), and bandConfidence itself defaults any non-finite/unknown input to medium.

The remaining changes are consistent and well-covered by new tests: fail-closed handling of parse_error abstentions across the query-PII tool, the CLI --checks pii path, and the composite check title; fail-closed piiColumnsFromReport (throws on a malformed report, caught and counted as a scanError so schema.detect_pii reports success: false); a per-file gradesByFile map that removes the multi-file grade race; and the export * as EngineCoerce self-namespace, which matches the repo's documented module convention.

Files Reviewed (12 files)

• packages/opencode/src/altimate/native/engine-coerce.ts
• packages/opencode/src/altimate/native/altimate-core.ts
• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/altimate/tools/altimate-core-check.ts
• packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-query-pii.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/altimate-core-check-formatters.test.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts
• packages/opencode/test/altimate/pii-detector-e2e.test.ts
• packages/opencode/test/cli/check-e2e.test.ts

Previous review (commit a00fb38)

Status: 2 Issues Found | Recommendation: Address before merge

Incremental review of commit a00fb3866 (round-3 polish since a03901b8). This commit cleanly fixes three prior reviewer concerns — it moves piiColumnsFromReport into engine-coerce.ts so tool imports no longer eagerly load the native NAPI binding at registry time, restores the missing-confidence PII band to medium (bandConfidence(null/undefined)), and makes the classify-pii error test's temp-directory cleanup leak-safe. No new issues were introduced by these changes. The two findings below are carried forward from the prior round and remain unresolved against current HEAD.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/opencode/src/altimate/native/altimate-core.ts	191	Composite check swallows a thrown checkQueryPii into pii = {}, which formatCheck renders as "No PII detected." — a false-clean PII verdict. The catch comment assumes the throw is for unparseable SQL, but the engine returns { parse_error, pii_columns: [] } (not a throw) for that; the throw path is for genuine internal failures (napi panic, schema-load error) that validation/lint may not report.

SUGGESTION

File	Line	Issue
packages/opencode/src/altimate/native/engine-coerce.ts	26	Shared bandConfidence still duplicates the local copy in review/runner.ts:62. This commit aligned the null/undefined default (both now medium), but the two copies still diverge for unrecognized non-null inputs (NaN -> "low" here vs 0.5 -> "medium" in the runner). Unify by importing the shared helper in runner.ts and deleting its local copy.

Files Reviewed (6 files)

• packages/opencode/src/altimate/native/engine-coerce.ts - 1 issue (carried forward, re-verified)
• packages/opencode/src/altimate/native/altimate-core.ts - 1 issue (carried forward, re-verified)
• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts
• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts

Fix these issues in Kilo Cloud

Previous review (commit a03901b)

Status: 2 Issues Found | Recommendation: Address before merge

Incremental review of commit a03901b8 (round-2 consensus fixes since e3db0a41). Most prior bot findings (PII confidence banding, semantics success flag, composite parse_error/query_targets, validate string suggestions, bytes labels, shared piiColumnsFromReport, deterministic classify-pii error test) are verified fixed. The two findings below are new residual issues introduced by this commit's changes.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/opencode/src/altimate/native/altimate-core.ts	191	Composite check swallows a thrown checkQueryPii into pii = {}, which formatCheck renders as "No PII detected." — a false-clean PII verdict. The catch comment assumes the throw is for unparseable SQL, but the engine returns { parse_error, pii_columns: [] } (not a throw) for that; the throw path is for genuine internal failures that validation/lint may not report.

SUGGESTION

File	Line	Issue
packages/opencode/src/altimate/native/engine-coerce.ts	26	New shared bandConfidence duplicates the pre-existing local copy in review/runner.ts:62 with a divergent missing-input default (NaN -> "low" vs 0.5 -> "medium"). pii-detector.ts and the runner's equivalence path now disagree on the band for a missing confidence. Unify by importing the shared helper.

Files Reviewed (16 files)

• packages/opencode/src/altimate/native/altimate-core.ts - 1 issue
• packages/opencode/src/altimate/native/engine-coerce.ts - 1 issue
• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/altimate/tools/altimate-core-check.ts
• packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-compare.ts
• packages/opencode/src/altimate/tools/altimate-core-policy.ts
• packages/opencode/src/altimate/tools/altimate-core-query-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-semantics.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/altimate-core-check-formatters.test.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts
• packages/opencode/test/altimate/altimate-core-semantics-formatters.test.ts
• packages/opencode/test/altimate/pii-detector-e2e.test.ts
• packages/opencode/test/cli/check-e2e.test.ts

Fix these issues in Kilo Cloud

Previous review (commit e3db0a4)

The review did not run because the selected model is no longer available.

Choose another model in Kilo Code review settings: https://app.kilo.ai/code-reviews

Previous review (commit e3db0a4)

Status: No Issues Found | Recommendation: Merge

Incremental review of commit e3db0a414 (since 6b16d7f49). The previous SUGGESTION on check.ts (suggested_masking: null leaking into suggestion) is fixed — (f.suggestion ?? f.suggested_masking ?? undefined) now coerces null → undefined, with a non-tautological regression assertion added. The as string | undefined cast remains required because both operands are unknown from Record<string, unknown>.

No new issues found in the changed code.

Files Reviewed (2 files)

• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/cli/check-e2e.test.ts

Previous review (commit 6b16d7f)

Status: 1 Issue Found | Recommendation: Address before merge

Fix these issues in Kilo Cloud

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
packages/opencode/src/cli/cmd/check.ts	200	suggested_masking: null leaks into suggestion as null

Files Reviewed (12 files)

• packages/opencode/src/cli/cmd/check.ts - 1 issue
• packages/opencode/src/altimate/native/altimate-core.ts
• packages/opencode/src/altimate/tools/altimate-core-check.ts
• packages/opencode/src/altimate/tools/altimate-core-migration.ts
• packages/opencode/src/altimate/tools/altimate-core-query-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts
• packages/opencode/src/cli/cmd/check-helpers.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts
• packages/opencode/test/altimate/altimate-core-native.test.ts
• packages/opencode/test/cli/check-e2e.test.ts
• packages/opencode/package.json
• bun.lock

---

_{Reviewed by glm-5.2 · Input: 77.6K · Output: 9.3K · Cached: 304.4K}

_{Review guidance: REVIEW.md from base branch main}

[cubic-dev-ai]

No issues found across 12 files

_{Re-trigger cubic}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:00d0a02467

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cubic-dev-ai]

All reported issues were addressed across 16 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:a03901b841

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (4)

packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts (2)

58-59: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Normalize confidence with bandConfidence.

The engine can return a numeric confidence. This line then renders text such as "0.85 confidence", while pii-detector.ts reports "high"/"medium"/"low" for the same engine field. Reuse bandConfidence from ../native/engine-coerce to keep one presentation.

♻️ Proposed change

-import { classificationToString } from "../native/engine-coerce"+import { bandConfidence, classificationToString } from "../native/engine-coerce"

- const confidence = f.confidence ?? "high"+ const confidence = bandConfidence(f.confidence)

🤖 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 `@packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts` around
lines 58 - 59, Update the confidence assignment in the classification flow to
import and reuse bandConfidence from ../native/engine-coerce, converting
f.confidence into the shared high/medium/low band while preserving the existing
high fallback when confidence is absent.

---

4-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider moving piiColumnsFromReport into engine-coerce.ts.

pii-detector.ts also pulls in the schema cache and the connector registry. Importing it from a tool module widens the tool's dependency graph for one pure coercion function. engine-coerce.ts is the existing home for engine shape normalization, and this file already imports it.

If you move the helper, keep a re-export from packages/opencode/src/altimate/native/schema/pii-detector.ts so packages/opencode/test/altimate/pii-detector-e2e.test.ts keeps its current import path.

🤖 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 `@packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts` around
lines 4 - 12, Move the pure piiColumnsFromReport helper into engine-coerce.ts
alongside the existing engine shape normalization utilities, and update
realPiiColumns to use that location. Preserve a re-export from pii-detector.ts
so the existing pii-detector-e2e.test.ts import path remains valid.

packages/opencode/src/altimate/tools/altimate-core-policy.ts (1)

32-36: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Consider a fallback verdict when allowed is absent.

If a future engine result omits both allowed and pass while the call succeeds, allowed is undefined and the title renders "VIOLATIONS FOUND" with an empty violations list. That reproduces the false-positive gate this PR fixes. Deriving the verdict from the violations count when the flag is missing keeps the output consistent with the body.

♻️ Proposed defensive fallback

- const allowed = (data.allowed ?? data.pass) as boolean | undefined+ const allowed = (data.allowed ?? data.pass ?? violations.length === 0) as boolean

🤖 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 `@packages/opencode/src/altimate/tools/altimate-core-policy.ts` around lines 32
- 36, Update the verdict logic in the policy result handling to fall back to the
violations count when both data.allowed and data.pass are absent, treating an
empty violations list as passing. Preserve the existing explicit allowed/pass
values and error title behavior, and ensure the title remains consistent with
the rendered violations body.

packages/opencode/src/altimate/native/schema/pii-detector.ts (1)

16-27: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Add an Array.isArray guard on columns.

If the engine returns a non-array columns value (for example an object keyed by column name), columns.filter throws a TypeError inside detectPii, and the surrounding catch swallows the scan silently. The consumer in altimate-core-classify-pii.ts already checks Array.isArray(data.columns) before calling this helper, so the check belongs here for consistency.

♻️ Proposed guard

 export function piiColumnsFromReport(piiData: unknown): Array<Record<string, any>> {
- const columns = ((piiData as Record<string, any>)?.columns ?? []) as Array<Record<string, any>>- return columns.filter((c) => c.classification !== "None")+ const columns = (piiData as Record<string, any>)?.columns+ if (!Array.isArray(columns)) return []+ return (columns as Array<Record<string, any>>).filter((c) => c?.classification !== "None")
}

🤖 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 `@packages/opencode/src/altimate/native/schema/pii-detector.ts` around lines 16
- 27, Update piiColumnsFromReport to validate that piiData.columns is an array
before filtering; fall back to an empty array for missing or non-array values,
while preserving the existing classification filter for valid arrays.

🤖 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 `@packages/opencode/src/altimate/tools/altimate-core-check.ts`:
- Line 66: Update the PII status logic in formatCheck so data.pii.parse_error
takes precedence over clean pii_columns and findings, returning an explicit
skipped or error title state instead of PASS. Preserve the existing PII detected
behavior when findings or columns are present and the normal PASS result when
the check completes cleanly.
In `@packages/opencode/test/altimate/altimate-core-e2e.test.ts`:
- Around line 1186-1191: In the PII classification test, add an assertion that
the computed piiCount is greater than zero before comparing it with
result.metadata.finding_count. Keep the existing equality and “: None” output
assertions unchanged so the test verifies a non-empty PII result and the
filtering behavior.
In `@packages/opencode/test/altimate/pii-detector-e2e.test.ts`:
- Around line 50-57: Update the test teardown in afterAll to close the
duck_pii_e2e connector before calling Registry.reset(), and preserve and restore
the prior ALTIMATE_TELEMETRY_DISABLED environment value rather than
unconditionally deleting it.
---
Nitpick comments:
In `@packages/opencode/src/altimate/native/schema/pii-detector.ts`:
- Around line 16-27: Update piiColumnsFromReport to validate that
piiData.columns is an array before filtering; fall back to an empty array for
missing or non-array values, while preserving the existing classification filter
for valid arrays.
In `@packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts`:
- Around line 58-59: Update the confidence assignment in the classification flow
to import and reuse bandConfidence from ../native/engine-coerce, converting
f.confidence into the shared high/medium/low band while preserving the existing
high fallback when confidence is absent.
- Around line 4-12: Move the pure piiColumnsFromReport helper into
engine-coerce.ts alongside the existing engine shape normalization utilities,
and update realPiiColumns to use that location. Preserve a re-export from
pii-detector.ts so the existing pii-detector-e2e.test.ts import path remains
valid.
In `@packages/opencode/src/altimate/tools/altimate-core-policy.ts`:
- Around line 32-36: Update the verdict logic in the policy result handling to
fall back to the violations count when both data.allowed and data.pass are
absent, treating an empty violations list as passing. Preserve the existing
explicit allowed/pass values and error title behavior, and ensure the title
remains consistent with the rendered violations body.

🪄 Autofix

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 Plus

Run ID: 5495fa1e-a85f-4b9f-8b58-c30f65d554c6

📥 Commits

Reviewing files that changed from the base of the PR and between e3db0a4 and a03901b.

📒 Files selected for processing (16)

• packages/opencode/src/altimate/native/altimate-core.ts
• packages/opencode/src/altimate/native/engine-coerce.ts
• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/altimate/tools/altimate-core-check.ts
• packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-compare.ts
• packages/opencode/src/altimate/tools/altimate-core-policy.ts
• packages/opencode/src/altimate/tools/altimate-core-query-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-semantics.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/altimate-core-check-formatters.test.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts
• packages/opencode/test/altimate/altimate-core-semantics-formatters.test.ts
• packages/opencode/test/altimate/pii-detector-e2e.test.ts
• packages/opencode/test/cli/check-e2e.test.ts

[cubic-dev-ai]

All reported issues were addressed across 9 files (changes from recent commits).

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@packages/opencode/src/altimate/native/engine-coerce.ts`:
- Around line 39-50: Update piiColumnsFromReport to validate that
piiData.columns exists as an array and that every row has the expected object
shape and classification field; return an explicit invalid-report result instead
of defaulting or silently filtering malformed data. In both cached and live
detection paths, propagate invalid reports as success: false while preserving
normal filtering of classification "None" for valid reports. Update the
malformed-report test and add coverage for missing columns, non-array columns,
and malformed rows.

🪄 Autofix

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 Plus

Run ID: 53f7c4f5-8e6a-41f1-92a9-382f2c1d9cf6

📥 Commits

Reviewing files that changed from the base of the PR and between a03901b and a00fb38.

📒 Files selected for processing (4)

• packages/opencode/src/altimate/native/engine-coerce.ts
• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:a00fb38668

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/opencode/src/altimate/native/engine-coerce.ts (1)

29-42: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Apply EngineCoerce.bandConfidence to PII columns output.

When data.columns is present, map f.confidence through EngineCoerce.bandConfidence. The current fallback displays missing confidence as "high" and numeric values as raw numbers. Keep the "high" fallback for legacy findings.

🤖 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 `@packages/opencode/src/altimate/native/engine-coerce.ts` around lines 29 - 42,
Update the PII columns output mapping to pass each column finding’s f.confidence
through EngineCoerce.bandConfidence, replacing the raw
numeric/missing-confidence handling so missing column confidence uses the
mapper’s medium default. Preserve the existing high fallback behavior for legacy
findings.

🤖 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 `@packages/opencode/src/cli/cmd/check.ts`:
- Around line 530-534: In the flat grade-field emission block around graded,
check the invocation’s input file count rather than graded.length. Emit
results.grade.grade and results.grade.score only when exactly one input file was
provided, while preserving grades for multi-file invocations even when only one
file has grade metadata.
---
Outside diff comments:
In `@packages/opencode/src/altimate/native/engine-coerce.ts`:
- Around line 29-42: Update the PII columns output mapping to pass each column
finding’s f.confidence through EngineCoerce.bandConfidence, replacing the raw
numeric/missing-confidence handling so missing column confidence uses the
mapper’s medium default. Preserve the existing high fallback behavior for legacy
findings.

🪄 Autofix

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 Plus

Run ID: d1c84d2f-e537-43c8-8bc4-13d38b77f93d

📥 Commits

Reviewing files that changed from the base of the PR and between a00fb38 and 960229d.

📒 Files selected for processing (12)

• packages/opencode/src/altimate/native/altimate-core.ts
• packages/opencode/src/altimate/native/engine-coerce.ts
• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/altimate/tools/altimate-core-check.ts
• packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-query-pii.ts
• packages/opencode/src/cli/cmd/check.ts
• packages/opencode/test/altimate/altimate-core-check-formatters.test.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts
• packages/opencode/test/altimate/pii-detector-e2e.test.ts
• packages/opencode/test/cli/check-e2e.test.ts

🚧 Files skipped from review as they are similar to previous changes (10)

• packages/opencode/src/altimate/review/runner.ts
• packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts
• packages/opencode/test/cli/check-e2e.test.ts
• packages/opencode/test/altimate/pii-detector-e2e.test.ts
• packages/opencode/test/altimate/altimate-core-check-formatters.test.ts
• packages/opencode/src/altimate/native/schema/pii-detector.ts
• packages/opencode/src/altimate/tools/altimate-core-query-pii.ts
• packages/opencode/src/altimate/tools/altimate-core-check.ts
• packages/opencode/test/altimate/altimate-core-e2e.test.ts
• packages/opencode/src/altimate/native/altimate-core.ts

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:960229d4ac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cubic-dev-ai]

All reported issues were addressed across 12 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[sahrizvi]

Approved — with a few optional follow-ups

Nice work on this one. I checked every claimed engine shape against the published @altimateai/altimate-core@0.7.0index.d.ts rather than against the inline comments, and the contract sync is accurate throughout: CompareResult, PolicyResult.allowed, EvalResult.overall_grade/scores.overall, SemanticResult.findings, LineageResult.queries[].edges, MigrationResult.findings[].risk, PiiReport/PiiColumnAccess, ThreatFinding.location, and ValidationError.location/suggestions all match. EquivalenceResult.differences genuinely still exists, so leaving those consumers alone was right. Version pinning is consistent across package.json and every bun.lock entry including the five platform optionals.

Two things worth calling out as done well: fixing runValidate's dead gate (result.success alone was making check --checks validate pass every file) is the highest-value correctness fix here, and the error-vs-verdict separation applied across the seven tools — never rendering SAFE/CLEAN/IDENTICAL/PASS on a failed engine call — is consistent and each case has a test.

Note on scope: this review was started against a00fb38668. Several findings were fixed by the commits that landed since (960229d4ac, 3974e35c93, 846cf24bc8, 4358bb4d46) and have been redacted rather than reported — specifically the composite-check PII abstention, the policy allowed: true suppression, the duplicate bandConfidence, the unguarded piiColumnsFromReport, the PII scan reporting success after swallowing failures, and the multi-file grade race. Everything below was re-verified against 4358bb4d46.

Nothing outstanding blocks merge. The items below are follow-ups, split by whether they came in with this PR.

---

Introduced by this PR

1. classificationToString fallbacks are inconsistent across call sites — the default is "PII" (altimate-core-check.ts:112, altimate-core-query-pii.ts:57, altimate-core-classify-pii.ts:57), "UNKNOWN" in pii-detector.ts:90,186, and "" in runner.ts:403. An unrecognized classification object therefore renders as a positive PII assertion in tool output and as an empty string in the review runner. "UNKNOWN" seems like the honest convention for all three.

2. pii-detector-e2e.test.ts fails hard where the other core tests skip — altimate-core-e2e.test.ts:19-24 guards on require.resolve("@altimateai/altimate-core") and uses describe.skip when the NAPI binary is absent. The new file imports pii-detector (which imports the package at module scope) and calls require("@altimateai/altimate-core") inside an ungated test, so without the binary it throws at import instead of skipping. Worth adding the same guard.

Related: the only test of detectPii itself is the DuckDB path behind ALTIMATE_RUN_WAREHOUSE_E2E === "1", so the pii-detector rewrite has no test that runs by default. A non-live unit test over the schema path would cover the biggest behavioural change here.

3. unbalanced_quote isn't a rule the 0.7.0 engine can emit — check-e2e.test.ts:610. SafetyRule is a closed union of ten values and this isn't one of them. The severity mapping and byte-range assertions in that test are genuine and valuable; just the rule name in the fixture isn't engine-faithful. tautology_attack would keep it real.

---

Pre-existing — not from this PR, but adjacent enough to be worth a follow-up

4. Policy remediation and warnings[] never reach output — PolicyViolation.remediation is not read anywhere, and runPolicy in check.ts maps f.suggestion, which PolicyViolation doesn't have, so every policy finding loses its fix hint. PolicyResult.warnings[] is likewise dropped by the CLI. formatPolicy now surfaces warnings on the pass path, so only the CLI side is left:

suggestion: (f.remediation??f.suggestion)asstring|undefined

5. Two direct engine calls still forward a raw empty dialect — sql/register.ts:186 (core.formatSql(params.sql, params.dialect)) and :465 (core.columnLineage(params.sql, params.dialect ?? undefined, …)). ?? doesn't coerce "", only || does, and siblings at :364/:433/:434 already use || undefined. Since ReviewConfig.dialect defaults to "", these are reachable and will throw unknown dialect ''. They fail closed rather than returning a wrong answer, so it's low urgency — but EngineCoerce.dialectHint would finish the job.

6. The review runner drops composite validation.errors and safety.threats — runner.ts:206-209 concatenates lint.findings, issues, violations, and findings, but the comment just above says validation failures surface via data.validation.errors, and nothing reads that or data.safety.threats. So validation errors and injection threats from the composite check don't reach the review output. Since this PR wired PII into the same block, it's a natural place to pick up the other two.

7. runSafety's dispatcher-failure path emits warning instead of error — every other check routes engine failures through dispatcherErrorFinding, which uses severity: "error" with the stated rationale "so CI doesn't false-pass". The safety fallback hardcodes "warning", so a safety-engine crash still passes --fail-on error.

8. runner.ts:293 reads the legacy grade field first — data.grade ?? data.overall_grade. Harmless today since data.grade is always undefined in 0.7.0, but it's the reverse of the ordering check.ts now uses.

---

Nits

• runSemantic's valid === false fallback emits a bare "Semantic check found issues" and drops SemanticResult.validation_errors, which carries the detail. extractSemanticsErrors already does this on the tool side.
• runSafety still maps line, column, and code, none of which exist on ThreatFinding — they're always undefined.
• The byte range prints an exclusive end as if inclusive: bytes 37-44 for [37, 7] actually covers 37–43. Worth noting this formula is now in two places, since the nested-findings mapper in runGrade copied it.
• normalizeSeverity leaves low → info untouched while explicitly reasoning about high and medium. Display is unaffected (--severity defaults to info), but a low-severity safety threat never trips --fail-on warning. Probably intended — a one-line comment would settle it.
• runPii dropping the numeric column field is correct (the engine's column is a name, not a position), but the name now lives only inside the message string, so JSON consumers can't key on it. A columnName field would help.

[anandgupta42]

@sahrizvi Thanks for the thorough review — especially for verifying against the published .d.ts rather than the inline comments. All follow-ups are addressed in 494407e9ad, with one respectful pushback:

Fixed (introduced by this PR):

1. ✅ classificationToString fallback unified to "UNKNOWN" everywhere (default changed; the runner's "" and the tools' "PII" call sites now use it).
2. ✅ pii-detector-e2e.test.ts now has the same NAPI-availability guard as altimate-core-e2e.test.ts — skips instead of crashing when the binary is absent. On the related note: the mapping itself (the biggest behavioral change) is covered by default-run tests via piiColumnsFromReport against a live classifyPii result; only the warehouse-introspection path stays behind the ALTIMATE_RUN_WAREHOUSE_E2E gate, consistent with the other DuckDB e2e.
3. ⚠️Pushback on "unbalanced_quote isn't a rule the 0.7.0 engine can emit" — it is, at runtime: scanSql("SELECT * FROM t WHERE name = 'x''") on the installed 0.7.0 binary returns threats: [{ rule: "unbalanced_quote", severity: "high", … }] (the 0.6.0 changelog added it, rule count 10 → 11). What's stale is the SafetyRule union in the published index.d.ts — filed upstream as AltimateAI/altimate-core-internal#764. Your adjacent point was still right though: the fixture's SQL would fire tautology_attack first, so I changed it to a dangling-quote payload that genuinely emits unbalanced_quote, with a comment citing the probe.

Fixed (pre-existing follow-ups):
4. ✅ runPolicy maps remediation into the suggestion. (warnings[] on the CLI side: the deny-path CLI never sees them since allowed !== false short-circuits; the tool-side pass path renders them as of 3974e35c93. Left the CLI pass path silent intentionally — happy to thread them as info findings if you'd prefer.)
5. ✅ Both remaining raw dialect forwards (sql.format, sql.column_lineage in sql/register.ts) now go through EngineCoerce.dialectHint.
6. ✅ Review runner threads composite validation.errors (as validate/error) and safety.threats into review issues.
7. ✅ runSafety's dispatcher-failure fallback now fails closed via dispatcherErrorFinding (error severity).
8. ✅ Runner grade reads overall_grade ?? grade.

Nits: all taken — validation_errors detail in the semantic fallback, dead line/column/code reads removed from the threats mapper, byte ranges now render the inclusive end ([37,7] → bytes 37-43, both call sites + tests), the low→info mapping documented as intentional, and PII findings carry a machine-readable columnName.

Full suite still green: 4782 pass / 0 fail, typecheck + marker check clean.

[cubic-dev-ai]

All reported issues were addressed across 7 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:494407e9ad

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:d4b12827d1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:27624151eb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cubic-dev-ai]

All reported issues were addressed across 5 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 90e2eae. Configure here.}

[cubic-dev-ai]

All reported issues were addressed across 6 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:90e2eaebd4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:b35057fbdb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".