You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(dbt-tools): say when schema-verify's verdict rests on a stale manifest - #1704
Closes#1367 (follow-ups from the review of #1368).
Type of change
Bug fix
What does this PR do?
For when the dbt project check fails and dbt-tools answers from the manifest of the last successful parse:
The schema-verify validator says so. Its message to the agent leads with that note; only the parse time comes from the answer, quoted through sanitizeForPrompt.
USAGE gives the stale shape of children and parents: { stale_manifest, models } instead of a list.
The lineage check reads the started adapter when there is one, so a dbt-integration that defines getModelLineage as an instance field isn't mistaken for one without it.
The next @altimateai/dbt-integration bump joins this PR once it clears the 3-day minimum release age.
Claims:
C1: When the validator fails and any answer carries stale_manifest, fixHint opens with the note. When none does, reason and fixHint are as on main.
C2: Of stale_manifest, only written_at reaches the message, flattened and quoted.
C3: dbtAdapterHasLineage() starts nothing, and is true for a started adapter whose getModelLineage is an own property.
R1: before any adapter starts, an instance-field getModelLineage goes unseen: only the prototype can be read.
How did you verify your code works?
Each change has a test that fails on main. Locally: bun turbo typecheck 13/13; dbt-tools 114 pass; opencode validators, dbt lineage and sql-query-deadline 868 pass, 0 fail. Tracker-leak check clean.
Cost side effects
None.
Checklist
I have tested my changes locally
I have not included unrelated changes in this PR
I checked for cost side effects (model calls, queries or cloud resources that grow with user data) and described them, or wrote "none"
Complexity delta (touched functions): cognitive unchanged; dbtAdapterHasLineage CCN 3 → 5; one new helper, cognitive 2.
Lineage support is now detected against the active dbt integration, including integrations that expose support at runtime.
Clarity
Help for the children and parents commands explains when results may include stale-manifest data.
Schema verification results now flag when they use the last successful parse’s manifest and include its timestamp when available. Current-manifest results remain unchanged.
…ifest
- The schema-verify validator's message leads with the stale-manifest note.
- `USAGE` gives `children`/`parents`' `{ stale_manifest, models }` answer.
- The lineage check reads a started adapter, so it finds an instance-field method.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VEmKQx8gR7vd6ettA3mWjP
The change updates dbt lineage capability detection to inspect an adapter instance when available. It also documents stale-manifest results and adds stale-manifest context to schema-verification guidance.
hasModelLineage accepts an optional adapter and checks its instance when provided. Tests cover instance-level and absent getModelLineage methods.
Active adapter wiring packages/opencode/src/altimate/native/connections/register.ts, packages/opencode/test/altimate/dbt-lineage-adapter.test.ts
The lineage check passes the cached adapter, or undefined, to the test hook and implementation. A test covers the check before and after adapter startup.
Stale-manifest reporting
Layer / File(s)
Summary
Stale-manifest result contract packages/dbt-tools/src/index.ts, packages/dbt-tools/test/cli.test.ts, packages/opencode/src/altimate/validators/dbt-schema-verify.ts
The children and parents descriptions document the stale-manifest result shape. Schema-verification results add optional stale-manifest metadata.
Failure guidance includes a stale-manifest notice when metadata is present. A string timestamp is sanitized and limited to 40 characters. Tests cover stale and current manifests, mismatch details, command hints, and timestamp formatting.
The reported test-isolation concerns do not block the supported test workflows. The change is mergeable after normal checks.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly identifies the main change: warning when schema verification relies on a stale manifest.
Description check
The description covers the issue, change, verification, cost side effects, and required checklist items. The screenshots section is not needed because this is not a UI change.
Linked Issues check
Issue [#1367] is closed and completed, so it provides historical context only. No active directly linked issue supplies coding requirements for this PR.
Out of Scope Changes check
The reviewed changes cover stale-manifest messaging and usage text, plus lineage detection for a started adapter. The added tests cover those same changes. These changes match the PR’s stated follow-u…
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Out of Scope Changes check
Explanation
The reviewed changes cover stale-manifest messaging and usage text, plus lineage detection for a started adapter. The added tests cover those same changes. These changes match the PR’s stated follow-up scope, and the summary identifies no unrelated change.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 8 files.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks the adapter’s way Then notes when manifests lag today A timestamp gets trimmed and neat The command hints make stale data clear And carrots mark the tests complete
@codex review against the numbered claims (C1–C3) and the disclosed residual (R1) in this PR's description: report only a reproducible trace that violates a numbered claim. Instances of the disclosed residual are not findings. A round with no claim violation ends review.
CI runs the suite in one process, where another file's mock of the dbt-tools adapter
module outlives that file, so this test can't rely on the real check. The seam now gets
the started adapter too; dbt-tools' own tests cover how the check reads it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VEmKQx8gR7vd6ettA3mWjP
@codex review against the numbered claims (C1–C3) and the disclosed residual (R1) in this PR's description: report only a reproducible trace that violates a numbered claim. Instances of the disclosed residual are not findings. A round with no claim violation ends review. Since the last clean round (7225c1c060), one commit changed: b87e3b650b passes the started adapter to the dbtAdapterInternals.hasModelLineage test seam too, and rewrites one test to assert which adapter the check is asked about.
return `The dbt project check failed, so schema-verify read the manifest of the last successful parse${written}. Its spec may not match schema.yml as it is now.\n\n${hint}`
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: The note can contradict the hint it now leads
The note says the spec may not match schema.yml as it is now, but the mismatch branch of the hint it prefixes still reads "Fix the model SQL to match the schema.yml spec (do not edit the spec), rebuild, and the harness will re-check". On a stale manifest an agent that follows that literally can remove a column the current schema.yml still requires; the post-rebuild re-check then runs against a fresh manifest and reports the opposite mismatch, so the session churns and may settle on the wrong column shape.
Consider giving the stale branch its own remediation — make the project check (re-parse) succeed first, then re-check — and keep the plain "match the spec" wording for a current manifest. The note itself is fine; it is the instruction under it that still assumes the spec is trustworthy.
The reason will be displayed to describe this comment to others. Learn more.
Not a break this PR introduces, so no change here. On main the same failure gives the same instruction ("Fix the model SQL to match the schema.yml spec…") with nothing to say the spec might be old. This PR only adds that warning above it. Telling the agent to do something different on a stale manifest, such as getting the project to parse first, would be a separate behaviour change, so I'm leaving it out of this follow-up.
`\n\nFix the model SQL to match the schema.yml spec (do not edit the spec), rebuild, and the harness will re-check before declaring done.`
: `Run \`altimate-dbt schema-verify <model>\` manually to diagnose the error. Check that altimate-dbt is on PATH and that the dbt project compiles cleanly.`,
The reason will be displayed to describe this comment to others. Learn more.
SUGGESTION: The stale verdict stays invisible to telemetry
details carries counts only, so the validator_check event cannot show how often this gate blocks on a manifest it has just told the agent not to trust. The sibling gate records its equivalent (stale_build in dbt-build-green.ts:483), so adding e.g. stale_manifest: results.some((r) => r.stale_manifest) beside errored / spawn_failures would keep this measurable without changing the agent-facing message.
The reason will be displayed to describe this comment to others. Learn more.
No change here: this is measurement, not a break. Also, stale_build in dbt-build-green is a different signal: models edited after their last build, not a stale manifest. A stale_manifest flag in details can come with a later change if we want to count these.
The new stale-manifest note leads a mismatch hint that still says "fix the model SQL to match the schema.yml spec", so an agent can edit a correct model to match a spec the note just called unreliable. The stale case should ask for a passing parse first.
The stale condition is not recorded in details, so validator_check telemetry cannot show how often the gate blocks on a stale manifest (the sibling dbt-build-green gate records stale_build).
Verified claims
C1 holds: leadWithStaleManifestNote runs only on the failing return, and returns the hint unchanged when no result carries stale_manifest. The ok: true paths are untouched.
C2 holds: only written_at from the answer reaches the message, string-checked, flattened by sanitizeForPrompt and quoted in guillemets. The answer's own message is ignored, so its prose cannot reach instruction position.
C3 holds: dbtAdapterHasLineage awaits no creation; dbtAdapter ?? undefined falls back to the prototype when no adapter is cached, and an own-property getModelLineage on the started adapter is now found. The dbt-lineage.ts:265 post-start guard is unchanged, so the pre-check remains an optimisation only.
R1 is as disclosed: before any adapter has started, an instance-field getModelLineage is still unseen.
Other checks (no findings)
sanitizeForPrompt flattens newlines/control characters and strips «», so the injected parse time cannot break out of its sentence.
hasModelLineage(adapter?) stays source-compatible for the two existing callers and both test seams; a throwing instance getter would be caught by the surrounding try.
The children / parents shape change needed no other consumer updates: no opencode code or bundled skill parses those answers, and graph.ts / failure-text.ts already handle the object answer.
New tests use the same PATH swap-and-restore pattern as onboarding/tool-detection.test.ts and the real-dbt E2E files, and the rewritten lineage test now avoids the shared mock.module leak; no new global mock or dispatcher state is left behind.
Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.
Use the tmpdir() fixture for this new test file.
This file uses a module-level os.tmpdir() with beforeAll and afterAll. The retrieved learning requires new test files in packages/opencode/test/altimate/ to import tmpdir from fixture/fixture.ts and use await using tmp = await tmpdir() per test.
The test also mutates process.env.PATH globally. Other test files in the same process can see that change while these tests run. Scope the stub directory to each test and restore PATH in the same scope.
Source: Learnings
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 6521b5da-b9da-415c-bfb7-8be024c45223
Commits
Reviewing files that changed from the base of the PR and between 3f3cae2 and b87e3b6.
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
On CodeRabbit's additional comment (dbt-schema-verify-stale.test.ts 29–54, marked unverified): no change, since it's not a break.
Temp dirs: the file follows the validator tests' own practice. Thirty files in test/altimate/validators use fs.mkdtemp, and none uses the tmpdir() fixture.
PATH: it's set in beforeAll and restored in afterAll, the same way the seven e2e-real-dbt* files do. bun runs test files one after another, so no other file runs while it's changed. The test passed in CI's single-process run on this head.
🤖 Code Review — OpenCodeReview (Gemini) — No Issues Found
No supported files changed.
This branch has not been deployed
No deployments
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
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.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Issue for this PR
Closes #1367 (follow-ups from the review of #1368).
Type of change
What does this PR do?
For when the dbt project check fails and dbt-tools answers from the manifest of the last successful parse:
sanitizeForPrompt.USAGEgives the stale shape ofchildrenandparents:{ stale_manifest, models }instead of a list.getModelLineageas an instance field isn't mistaken for one without it.The next
@altimateai/dbt-integrationbump joins this PR once it clears the 3-day minimum release age.Claims:
stale_manifest,fixHintopens with the note. When none does,reasonandfixHintare as on main.stale_manifest, onlywritten_atreaches the message, flattened and quoted.dbtAdapterHasLineage()starts nothing, and is true for a started adapter whosegetModelLineageis an own property.getModelLineagegoes unseen: only the prototype can be read.How did you verify your code works?
Each change has a test that fails on main. Locally:
bun turbo typecheck13/13; dbt-tools 114 pass; opencode validators, dbt lineage andsql-query-deadline868 pass, 0 fail. Tracker-leak check clean.Cost side effects
None.
Checklist
Complexity delta (touched functions): cognitive unchanged;
dbtAdapterHasLineageCCN 3 → 5; one new helper, cognitive 2.🤖 Generated with Claude Code
https://claude.ai/code/session_01VEmKQx8gR7vd6ettA3mWjP
Summary by CodeRabbit
childrenandparentscommands explains when results may include stale-manifest data.