Name the contracts the reviewer cannot read, before it guesses at them

[jschatz1]

First slice of the retrieval work. It turned out the premise needed correcting, so this closes the gap it can actually close and lays the parsing for the rest.

The case

CLI #95 and TUI #92 (2026-09-08) both moved kai-engine forward and switched four call sites onto kaipath.UserPath. Neither review could read that function — 17 lines, in another module — and both spent most of their output on it anyway: whether it is variadic, whether zero trailing components are supported, whether an empty override preserves the default, whether the one-argument call is valid. TUI #92 escalated the arity question to "That's a real defect, not a…".

Every one of those was speculation about a file nobody could open, rendered indistinguishably from the findings around it.

The reviewer was not being careless. It is told to ground every claim, and it was handed a diff whose correctness genuinely rests on a contract it had no way to see. What it lacked was permission to say so once and move on.

Why this does not fetch the source

I checked, and the original plan was wrong about this. The review pod is debian:bookworm-slim carrying kai and kit — no Go toolchain, no module cache — and the workflow never runs go mod download, which for a private module would need GOPRIVATE and auth besides. Sibling module source is not merely unfetched, it is absent. A "just read the pinned file" change would have had nothing to read.

So this names the gap instead of pretending to close it:

DEPENDENCIES THIS DIFF MOVES (resolved from the diff before this review started).
You cannot read these. They are other modules, this checkout holds none of their
source, and no tool you have will open them — do not spend a turn trying.
  github.com/kaicontext/kai-engine  v0.6.58 -> v0.6.59-0.20260908192613-5ab1b102fc2f (commit 5ab1b102fc2f)
    imported here: kaipath

An unread contract is a LIMITATION, not a defect. Say it ONCE — name the module and
what rests on it — then review what you can actually see. Do not open a concern per
call site, do not ask the author to confirm a signature you could not read, and do
not call it a defect: you have no evidence either way, and a reader cannot tell that
speculation from the findings around it.

The instruction is the load-bearing half. The list alone would only tell the reviewer what it already knew it could not see.

Groundwork, not a detour

The parsing is exactly what a real fetch needs: module path, version, and the commit inside a pseudo-version. TUI #92's diff carried v0.6.59-0.20260908192613-5ab1b102fc2f — the precise source that would have answered every one of its questions, sitting in the input it was handed. When there is a way to read it, that commit is already resolved.

Scope

Reads go.mod only — go.sum restates every version and would double each entry. A module appearing only on a - line was removed, and a removed dependency is not a contract the change rests on. Modules the diff actually imports sort first, so a transitive bump never buries the one the code reaches. Bounded at 6 modules and 5 packages each.

Verified

Five tests built on the real TUI #92 diff: the module/version/commit/package extraction, go.sum exclusion, removals and no-ops skipped, pseudo-version parsing including three shapes that must not yield a commit, and the block's content. Block assertions run against whitespace-collapsed text so they check what it says, not where it wraps. go test ./cmd/... green (exit 0).

🤖 Generated with Claude Code

[greptile-apps]

jschatz1 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: bab712a2-f589-4e3c-b5f5-385275dca748

---

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.}

[kaicontext]

Kai review

Kai Summary

Nothing I'd call a defect, but 1 decision to say yes to. 👇

Scope: reviewing kaicontext/kai-cli at the merged commit (19c2db1). The change is fully contained in two new files (cmd/kai/review_commit_deps.go, cmd/kai/review_commit_deps_test.go) and one one-line call site at cmd/kai/review_commit.go:678. Nothing outside this repo is read; the Go pseudo-version format that rcPseudoCommit parses is an external fact I reasoned over but did not fetch.

What it does, and my take: This adds a pre-review prompt step that parses go.mod diff hunks, extracts which module versions moved (and the 12-hex commit inside any pseudo-version), lists the packages the diff imports from those modules, and emits a prompt block telling the review agent that an unread sibling-module contract is a limitation, not a defect — say it once and move on. The motivation is concrete and stated: two prior reviews (CLI#95, TUI#92) burned output speculating about kaipath.UserPath, a 17-line function in another module they couldn't open. The code is careful about the edge cases I can construct, and the tests genuinely fail on the unfixed code. This is a sound prompt-engineering change with no defects.

No real concerns. I walked the recurring failure shapes against this diff:

• Index/nil safety: line[6:] is guarded by HasPrefix(..., "+++ b/") / "--- a/" (both 6 chars). line[0] access is guarded by the len(line) == 0 check. moved[mod] is initialized before assignment. Safe.
• File tracking: inGoMod resets on every +++ b/, --- a/, and diff --git line. go.sum is correctly excluded — the diff --git line sets it false (suffix go.sum ≠ go.mod), and the ---/+++ lines confirm it. TestChangedDepsReadsOnlyGoMod would fail if the guard were removed.
• Removed/unchanged deps: TestChangedDepsSkipsRemovedAndUnchanged catches the bug — remove the v.to == "" || v.to == v.from guard and both old/gone (removal) and same/pinned (no-op) enter out, failing len(deps) == 0. This is a real test, not a discards-the-answer test.
• rcPseudoCommit: The 12-hex-char check and len(parts) < 3 guard cover plain semver, two-part pre-releases, and non-hex suffixes. A pathological pre-release like v1.2.3-rc.1-abcabcabcabc would falsely yield a commit, but that isn't a valid Go module version in practice and the consequence (naming a commit in a limitation block) is benign.
• Empty case: rcDepLimitsBlock(nil) returns "", so the call site at review_commit.go:678 is a clean no-op for diffs touching no dependencies, and TestDepLimitsBlockSaysItOnceOrNotAtAll verifies this. The trailing \n\n properly separates from INTENT:.
• Callers: Within kaicontext/kai-cli, the only non-test caller of rcChangedDeps and rcDepLimitsBlock is review_commit.go:678; rcImportedPkgs is called only from review_commit_deps.go:144. Sibling repos or binaries I cannot see don't appear to reach these main-package symbols, but I can only state that within this repo.

DECISIONS:

• The injected instruction — "do not call it a defect: you have no evidence either way" and "do not open a concern per call site" — is a review-policy choice that changes what every future dependency-bumping review reports. It affects every review whose diff moves a go.mod dependency: where the agent previously produced per-call-site speculation (sometimes escalated to a "real defect"), it will now emit one scoped LIMITATION sentence and stop. This is the author's intent and the right call for the failure mode it targets, but it also suppresses the rare case where a contract violation is detectable from the diff alone (e.g., a call site passing the wrong argument count to a function whose signature changed between two pseudo-version commits both visible in the diff). Scoping the instruction to "an unread contract" rather than "any dependency change" mitigates this, and the agent is still told to "review what you can actually see." The author should own that this tradeoff is now live for every review run.

Merge it. The code is correct, the tests verify the behavior they claim to, and the one policy decision above is the author's own and clearly intended.

Important files changed

File	Change
cmd/kai/review_commit.go	modified · +4 −0
cmd/kai/review_commit_deps.go	modified · +232 −0
cmd/kai/review_commit_deps_test.go	modified · +121 −0

What I opened — 4 files, 12 turns, 9m26s

• cmd/kai
• cmd/kai/review_commit.go
• cmd/kai/review_commit_deps.go
• cmd/kai/review_commit_deps_test.go

Decisions

Correct as written, but somebody should say yes to these:

• The injected instruction ("do not call it a defect… do not open a concern per call site") is a review-policy choice that changes what every dependency-bumping review reports going forward — it suppresses per-call-site speculation about unread contracts, which is the intent, but it also suppresses the rare case where a contract violation is detectable from the diff alone; the author should own thi…

_{+357 −0 · 3 files · reaches 11 · the full analysis
💬 Reply to any of my comments and I'll answer, or say @kaicontext anywhere on this PR — a question, or "take another look at the retry logic".}