Repository navigation
fix: a cut-off altimate-code answer is a failure, and release 1.4.1 - #17
Conversation
Invalid JSON with exit 0 counted as ok with no metadata, so the graph lookup cached "no model" for the session and the edit's impact note silently went missing. It now reports a failure and asks again next time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNQjkaiykbC1WtASw6Yegs
Lineage shipped under 1.4.0, so `plugin update` tells an earlier 1.4.0 install it is already current. The bump lets those installs pick it up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNQjkaiykbC1WtASw6Yegs
|
Review log: claims for this PR
Accepted residuals
|
|
@codex review against the numbered claims and the disclosed residuals in the review-log comment on this PR: report only a reproducible trace that violates a numbered claim. Instances of the disclosed residuals are not findings. A round with no claim violation ends review. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
The engine gives it to every column read from a CTE or a qualified table, so to all of a dbt model's, where it reads as doubt about each column. Real labels still print. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNQjkaiykbC1WtASw6Yegs
|
@codex review against the numbered claims and the disclosed residuals in the review-log comment on this PR: report only a reproducible trace that violates a numbered claim. Scoped round: review only the new commit a77324a, which is covered by claim C5. Instances of the disclosed residuals are not findings. A round with no claim violation ends review. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
sahrizvi
left a comment
sahrizvi
left a comment
There was a problem hiding this comment.
Requesting changes for one major finding: on an edit, a failed read still produces no note and no message, which the changelog and description list as a symptom of the old behaviour. The cut-off report in the pane, the "Not sure" suppression and the release itself all hold up.
The inline comments carry the major and four minors, none of which blocks by itself. One more minor and the nits are in a separate PR comment.
Checked at base and at head a77324a0e7: claude plugin test gives 157 pass at base and 159 at head, claude plugin validate --strict passes for the marketplace and the plugin at both, and tsc reports 0 errors at both.
| const data = parseJson<{ status?: string; output?: string; metadata?: unknown }>(result.stdout) | ||
| // An answer that is not JSON (an altimate-code that exits before a pipe drains cuts it off) is a failure | ||
| // to report and ask again, not "no such model". | ||
| if (result.exitCode === 0 && !data) return { metadata: null as any, text: `${name}'s answer could not be read`, ok: false } |
There was a problem hiding this comment.
Major: a failed read still leaves the edit with no note, and nothing says so.
This new failure reaches impactFirst as a throw from graphFromTools, and impactFirst drops it (graphFromTools(...).catch(() => null) at line 882) and returns null. The toast at line 1174 fires only when the lookup did not finish in time, so a failure that did finish is silent.
I ran an Edit on a 6 MiB manifest with every CLI answer cut off. At this head, result.context is empty, there are 0 toasts and the band is empty. The pane message is written, but a follow that fails returns before $.ui.open (ui.open ran 0 times on a cut-off read and once on a good read), so with the pane closed nothing shows.
The changelog (line 12) and the PR description both list "left the edit with no note" among the "before" symptoms. If that read succeeds, the next edit does get a note. A cut-off can be persistent, though: released 0.12.9 on a synthetic 18 MB manifest, with tool run dbt_lineage ... --json | cat, gave exit 0, 98,280 bytes and invalid JSON on 11 of 11 runs for the same model. Every edit of a model like that goes ahead with no note and no word.
This is not a regression, since the base has no note either. I rate it major because it is this PR's stated before and after, on the safety path, and the timeout path right next to it does tell the user.
Suggested fix: record the failure in progress at line 882, toast that the lookup failed and the edit went ahead without it, and add a line to Claude's context saying downstream readers are unknown. If silence is intended, drop the "no note" clause from the changelog and the description.
There was a problem hiding this comment.
Fixed in 74b0370. impactFirst records the failure, a toast says "the lineage lookup failed; the edit went ahead without it", and the edit's context tells Claude what reads the model is unknown. I kept the changelog's claim rather than dropping it. Checked: the new Edit test fails on the previous head. On a real edit with released 0.12.9 and a 495 KB answer, a real cut-off hit the seventh edit: the log said the lookup failed, and Claude's reply said the edit went ahead without checking lineage. A file the manifest doesn't know stays silent, with a test.
| const data = parseJson<{ status?: string; output?: string; metadata?: unknown }>(result.stdout) | ||
| // An answer that is not JSON (an altimate-code that exits before a pipe drains cuts it off) is a failure | ||
| // to report and ask again, not "no such model". | ||
| if (result.exitCode === 0 && !data) return { metadata: null as any, text: `${name}'s answer could not be read`, ok: false } |
There was a problem hiding this comment.
Minor: valid JSON of the wrong shape still sticks as "no model".
!data catches only what does not parse. With exit 0, {"status":"ok"}, {}, [], "x" and 12 each give ok: true with null metadata, and graphFromTools caches that (1 CLI call after 2 reads). null, 0 and empty output get the new failure.
Not a blocker, because this is edge input. tool run --json writes one compact line, so a cut-off prefix can never parse (checked on a real cut), and the engine always writes the whole object.
Suggested fix: require an object with a string status.
There was a problem hiding this comment.
Fixed in 74b0370. runTool now requires an object with a string status. {"status":"ok"} passes that, so graphFromTools also treats empty metadata as a failure whether or not the call was ok. Checked: a test for each of your eight inputs, five of which fail on the previous head. A really missing model still says "No dbt model matches" on the real CLI.
| } | ||
| const data = parseJson<{ status?: string; output?: string; metadata?: unknown }>(result.stdout) | ||
| // An answer that is not JSON (an altimate-code that exits before a pipe drains cuts it off) is a failure | ||
| // to report and ask again, not "no such model". |
There was a problem hiding this comment.
Minor: no cooldown after a failure.
With a persistent cut-off, 5 reads make 5 CLI runs (1 at base), each failing edit makes 2 (one before it, one for the follow after it), and a shell command naming 5 files makes 6 per command.
Not a blocker. Asking again is what this PR promises, there is no timer or self-loop, in-flight calls are shared and the edit hold stays capped at 8.5 s. Each run cost 1.3 s warm and 2.7 s cold on an 18 MB manifest, and the engine fix removes the trigger. A short per-key cooldown is safe to do in a follow-up.
There was a problem hiding this comment.
Agreed, leaving this for a follow-up, as you suggest. It goes on the follow-up list, and the CLI fix removes the trigger. It's also recorded in the review log as a residual.
| - A manifest over 4 MiB is read through altimate-code. When the answer arrives cut off, the pane now reports that the answer could not be read, and asking again tries again. Before, it said "No dbt model matches" for a model that exists, kept saying so until the manifest changed, and left the edit with no note. altimate-code releases up to 0.12.9 can cut off a large answer on a pipe. | ||
|
|
||
| ### Upgrading | ||
| - Installs of 1.4.0 made before this release have no lineage, because `plugin update` compares versions. In a terminal, run `claude plugin marketplace update altimate-claude-plugin`, then `claude plugin update altimate-code@altimate-claude-plugin`, then restart Claude Code. |
There was a problem hiding this comment.
Minor: this line is too broad.
"Installs of 1.4.0 made before this release have no lineage" is not true of installs made between the lineage merge and this release; those have it. "Made before lineage merged" would be accurate.
These commands also apply only to this marketplace. The README also documents claude plugin install altimate-code from the plugin directory, which I did not test.
Not a blocker: this is wording, safe to fix in a follow-up. The commands and the marketplace name are right. In an isolated config directory with a local marketplace, a pre-lineage 1.4.0 install stayed "already at the latest version (1.4.0)" at base, and updated from 1.4.0 to 1.4.1 with lineage at this head.
Listing lineage under 1.4.1 "Added" is fine: its first commit is dated 2026-11-02, so the 1.4.0 section dated 2026-10-28 would be wrong too.
There was a problem hiding this comment.
Fixed in 74b0370: it now says "A 1.4.0 install from this marketplace made before lineage merged". I didn't test the README's install from the plugin directory either, so the line doesn't cover it.
| await ui.unmount() | ||
| }) | ||
|
|
||
| test('an answer cut off on its way is a failure, not "no such model": it is not remembered, and the next look asks again', async ($, on) => { |
There was a problem hiding this comment.
Minor: the edit path on a cut-off is untested.
This test covers Read only. Not covered: Edit with a cut-off graph read (the case in the major finding), a cut-off impact_analysis (the column goes to "not checked"; my probe shows that works), a cut-off plain dbt_lineage, a non-zero exit with cut-off stdout, and wrong-shape JSON.
Not a blocker: test-only. The two new tests are sound, though. Reverting lineage.tsx fails both, reverting each label hunk alone fails the label test, and the fixture has the shape of a real cut (compact prefix, no newline, exit 0).
There was a problem hiding this comment.
Added in 74b0370: an Edit with a cut-off graph read, a cut-off impact_analysis (column "not checked"), a cut-off plain dbt_lineage, a non-zero exit with cut-off stdout, and wrong-shape JSON. claude plugin test is at 172 pass. The impact_analysis and non-zero cases also pass on the previous head, as your probe found; they're there as guards.
|
Notes from the review of Minor, not tied to a diff line: the README still promises a derivation that now rarely shows. Nits
|
…wer object counts as an answer The failure reached the edit as silence: now a toast says the edit went ahead, and Claude is told what reads the model is unknown. Valid JSON of the wrong shape no longer sticks as "no model". The changelog and tests follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YNQjkaiykbC1WtASw6Yegs
|
On the notes in your PR comment, as of
|
|
@codex review against the numbered claims and the disclosed residuals in the review-log comment on this PR: report only a reproducible trace that violates a numbered claim. Scoped round: review only the new commit |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
sahrizvi
left a comment
sahrizvi
left a comment
There was a problem hiding this comment.
Approved at 74b0370. I checked the commit added since my last review directly; this was not a full panel re-run.
The major finding is fixed: an edit whose lineage lookup fails now goes ahead with a toast, and Claude is told that what reads the model is unknown.
What I ran:
claude plugin test: 172 pass.claude plugin validate --strict: passes.- Undoing the toast fix fails its test, and undoing the answer-shape check fails 5 tests.
- Making an empty answer a failure is safe: the real engine still returns metadata for a missing model and a missing manifest, so "No dbt model matches" keeps working.
tsc was not run on this commit.
Minors left, neither blocking:
- No cooldown after a failed read: retrying is what the PR promises, and it's limited by Claude's own tool calls, so it's safe to do in a follow-up.
- The README still promises "how it is derived", though dbt columns now rarely show a label: documentation only, safe to do in a follow-up.
What changes for users
plugin updatetold an older 1.4.0 install it was already current. 1.4.1 ends that.Why
altimate-code up to 0.12.9 can cut off a large
--jsonanswer on a pipe (fix: AltimateAI/altimate-code#1687). The plugin took invalid JSON with exit 0 as a success with no metadata, and cached that as "no model".This PR doesn't stop the cut-off. With 0.12.9, big answers can still fail, but they say so. The altimate-code fix stops them failing. Neither PR needs the other to merge.
Verification
/lineageon the two largest answers I tested, on an 11 MB manifest: 15 drew the graph, 9 showed the failure, 0 said "no model".claude plugin updatetook an install from before the lineage merge from 1.4.0 to 1.4.1, with lineage in it. On 1.4.0 it reported "already at the latest version".claude plugin test: 172 pass.claude plugin validateand typecheck pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01YNQjkaiykbC1WtASw6Yegs