fix: redact non-SQL content echoed in safety threat messages (main sanity failure) - #1111

…findings on 5-PR corpus) (AltimateAI#1029)

* feat(review): [R20 S1] grain-key `not_null` completeness detector

For every column named in a `dbt_utils.unique_combination_of_columns`
test's `combination_of_columns`, require `not_null` coverage on the
same model. Otherwise a NULL grain-key value silently passes the
uniqueness test — the guardrail is toothless. Cited as a hard rule in
`docs/DBT_GUIDELINES.md` in the corpus repo; kilo catches it, we didn't.

Coverage sources:
- `constraints: [{type: not_null}]` — only counted when
  `contract.enforced == true` (per codex R20 S1 high AltimateAI#3). On views /
  non-contracted models, `constraints:` is documentation-only, not
  enforced, so we don't miss a real gap.
- Column-level `data_tests: [not_null]` / `tests: [not_null]` — always
  counted; dbt's test runner enforces regardless of contract state.

Recommendation flips between `constraints:` (contracted model) and
`data_tests:` (view / non-contracted) based on the model's contract
state — matches the adapter-semantics discussion in the corpus study
(PR D×2 + PR A×2).

Supported YAML shapes:
- `unique_combination_of_columns` and `dbt_utils.unique_combination_of_columns`
  (exact match per codex high AltimateAI#1; `endsWith` would over-match).
- pre-1.9 flat args + dbt 1.9+ `arguments:` nesting.
- top-level `contract:` and nested `config.contract:` for enforcement flag.

False-positive guards:
- Column-name comparison is case-folded (Snowflake identifiers, per
  codex high AltimateAI#2) so `WORKSPACE_ID` in `combination_of_columns` matches
  `workspace_id` in `columns:`.
- Skipped on `status === "deleted"` files (no current grain to guard).
- Only runs on files in the PR diff, not repo-wide.

### Validation on 5-PR internal corpus (recall improvement)

vs S4 baseline (the tier-promotion PR this branch stacks on):
| Variant | S4 baseline | S1 result | Delta |
|---|---:|---:|---:|
| A1 (`--pure --no-ai=true`) | 118 | 129 | **+11 grain-key gaps** |
| A2 (`--pure`, LLM lane) | 143 | 150 | +7 (net; LLM run-to-run noise) |

Grain-key gaps caught:
- PR B (AltimateAI#1111) ×1: `mrt_all_purpose_auto_tune_recommendation.rec_type`
- PR C (AltimateAI#862) ×10: workspace_id x3, job_id x2, period_start_time x2, period_end_time x2, task_key x1
- PR A, D, E: 0 (fixes already landed in HEAD state per corpus study)

Strong matches to human blocker findings on PR C:
- `int_job_run_billing.workspace_id`, `int_job_run_cost_carrier.workspace_id`,
  `int_job_task_run_cost_carrier.workspace_id` → PR C human F11 (critical:
  "workspace_id missing from carrier identity")
- `mrt_job_run_timeline.period_end_time`, `mrt_job_task_run_timeline.period_end_time`
  → PR C human F10 (critical: "dbt mart grain includes period_end_time")

### Tests

- 71 pass / 0 fail in `review-dbt-patterns.test.ts` (58 pre-existing + 13 new S1 tests).
- 3797 pass / 640 skip / 0 fail in the full altimate review suite (132 files).
- Codex-reviewed diff, 3 highs addressed:
  - endsWith → exact-name match
  - column-name case-folding
  - constraints only count when contract enforced
- Codex minor AltimateAI#4 addressed (test fixture for top-level `contract:`)
- Codex minor AltimateAI#5 addressed (test fixture for non-contracted constraints ≠ coverage)

Depends on PR AltimateAI#1028 (feat/review-r20-s4-triage-promotion), which in turn
stacks on PR AltimateAI#1027 (feat/review-r18-observability-recall).

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Claude-Session: https://claude.ai/code/session_017zXDXMiNFh4qDPxPCfa2of

* fix(review): [R20 S1] address consensus-review findings on grain-key detector

Bundle addresses PR AltimateAI#1029's consensus review:

- MAJOR AltimateAI#1 — model-level and column-level primary_key / not_null
  constraints now count as coverage. dbt 1.5+ supports model-level
  constraints via `constraints: [{type: primary_key, columns: [a, b]}]`,
  and a primary_key inherently enforces NOT NULL on Postgres/Snowflake/
  BigQuery/Databricks. Grain columns declared via a model-level PK were
  previously falsely flagged as missing not_null. Fix scans both
  `mm.constraints` (model-level, honouring the `columns:` list) and
  column-level `constraints: [{type: primary_key}]`.

- MINOR AltimateAI#3 — contract-precedence bug. Earlier ternary short-circuited
  when `cfg.contract` was any object (e.g. `config: {contract:
  {alias: X}}` with no `enforced` key), masking a top-level
  `contract: {enforced: true}`. Now evaluated independently at both
  locations and OR'd.

- MINOR AltimateAI#4 — `dbt.` prefix on namespaced test names (`dbt.not_null` in
  dbt 1.8+) is stripped in `testName()` before matching, so it counts
  as coverage.

- MINOR AltimateAI#6 — `{name: <alias>, test_name: not_null}` alternative object
  form is recognised. Reading `Object.keys(t)[0]` returned `name`
  (the alias) rather than the underlying test type. Now `test_name`
  wins when present, falling back to the first key.

- NIT AltimateAI#7 — `norm()` hoisted from per-model to function scope.

Consensus items NOT addressed this round:
- NIT AltimateAI#8 (dedup GrainKeyGap for a column listed twice / multiple
  grain tests) — collapses downstream via the global finding
  fingerprint; cosmetic rather than correctness.
- NIT AltimateAI#9 (model-level `data_tests:` scanned as a grain-test source)
  — reviewer noted "harmless (no false match)"; no action.
- NIT AltimateAI#10 (documentation of fallback-skip) — no code change needed.
- MINOR AltimateAI#5 (contract resolved from dbt_project.yml or SQL config)
  — cross-file / cross-context, deferred.

Regression tests (all pass; 82 pass / 0 fail in review-dbt-patterns
suite, 3828 pass / 0 fail in full altimate suite — up from 3785):

- Model-level primary_key constraint covers grain columns
- Model-level not_null constraint with `columns:` list covers named cols
- Column-level primary_key counts as coverage
- Precision guard: PK missing cols still flagged
- Model-level constraints on non-contracted model don't count
- `config.contract` without `enforced` does NOT mask top-level
  `contract: {enforced: true}` (MINOR AltimateAI#3)
- `dbt.not_null` covers (MINOR AltimateAI#4)
- `{name:, test_name: not_null}` alternative form covers (MINOR AltimateAI#6)

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Claude-Session: https://claude.ai/code/session_017zXDXMiNFh4qDPxPCfa2of

* chore(review): [R20 S1] genericize internal-vocab leaks in dbt-patterns comments

Companion cleanup to PR AltimateAI#1028's FinOps strip. The grain-key detector's
docstrings named a specific internal column (`workspace_id`) as the
Snowflake case-folding example and referenced internal corpus PR labels
(`PR D×2, PR A×2`). Neither carries product meaning outside the
altimate-ingestion codebase.

- `dbt-patterns.ts:962` — replace `WORKSPACE_ID` / `workspace_id` example
  with `ORDER_ID` / `order_id`. Same illustrative point, no internal name.
- `dbt-patterns.ts:1443` — replace `(PR D×2, PR A×2)` corpus-label
  citation with `(four instances across the sample)`. Same count, no
  internal reference.

Tests: 250/250 green.

* fix(review): [R20 S1] address altimate-harness-bot review findings on grain-key detector

Two substantive findings on PR AltimateAI#1029:

1. `dbt-patterns.ts:1099` — grain detector was not change-scoped.
   `extractGrainKeyGaps(newDoc)` ran against every entity in the file,
   so a housekeeping edit (description bump, meta tag) surfaced all
   pre-existing grain gaps on unrelated models. Real precision cost:
   reviewers seeing findings on a PR they didn't intend suppress the
   whole rule.

   Fix: compare the `combination_of_columns` set per entity between
   `oldDoc` and `newDoc`; only emit gaps for entities whose grain
   declaration actually changed (added, removed, or column set diff).
   Added entities on the new side count as changed; the "added file"
   case (no oldDoc) unconditionally treats every entity as changed
   so newly-shipped grain declarations are still guarded.

   New helpers: `extractGrainDeclarations` returns
   `Map<entityName, Set<column>>` and `grainDeclChangedEntities` diffs
   two docs into a set of entity names whose declaration moved.
   Existing test that pinned the old steady-state-fires behavior
   (`R20 S1: unique_combination_of_columns with grain col missing
   not_null → warning finding`) rewritten to test the newly-added
   case; added a companion test that pins the new precision
   guarantee (`steady-state grain gap on unchanged model is NOT
   re-surfaced`), plus a `grain declaration changed (column added to
   combination_of_columns) does fire gap` test that keeps the
   regression case covered.

2. `dbt-patterns.ts:963` — `extractGrainKeyGaps` iterated only
   `d.models`, silently skipping `snapshots`, `sources`, and `seeds`.
   `unique_combination_of_columns` on a snapshot is a real SCD-2
   grain declaration; sources declare per-table `columns:` + tests;
   seeds carry the model shape.

   Fix: new `iterateGrainEntities(d)` helper walks all four sections
   (mirrors `extractTestOccurrences`). Sources descend one level to
   iterate their `tables[]` entries which carry the model shape.
   Three new tests: `grain detector also covers snapshots`,
   `... source tables`, `... seeds`.

Tests: 255/255 green (8 review-* files, +6 new grain-key tests).

* fix(review): [R20 S1] qualify source-table entity names as `<source>.<table>` (cubic-review P2)

Two sources both containing a table named `orders` (e.g. `raw.orders`
and `legacy.orders`) previously conflated in `iterateGrainEntities`:
- Grain-change detection collapsed them into a single map entry, so a
  change in one source's grain declaration could surface or suppress
  gaps for the other.
- Finding fingerprint uses the entity name; two distinct source tables
  with the same table name would dedupe to a single finding.

Fix: `iterateGrainEntities` now returns `{name, body}` pairs. Source
tables are qualified as `${sourceName}.${tableName}` while models,
snapshots, and seeds retain their unqualified name (they live in a
flat namespace already).

Tests: `grain detector also covers source tables` updated to assert
the qualified name; new test `same source-table name in two sources
does NOT conflate` locks in the fix.

257/257 review-* tests pass.

---------

Co-authored-by: Haider <[email protected]>
Co-authored-by: Claude Opus 4.7 <[email protected]>