Skip to content

State the declared Go version in review prompts as a fact - #1332

Merged
wesm merged 5 commits into
kenn-io:mainfrom
subaru-ye:prompt/toolchain-version-fact
Oct 16, 2026
Merged

wesm merged 5 commits into
kenn-io:mainfrom
subaru-ye:prompt/toolchain-version-fact

Conversation

@subaru-ye

@subaru-ye subaru-ye commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review prompts now state the Go version declared by each module touched by Go source or go.mod changes. This gives reviewers a concrete compatibility baseline instead of relying on memory. Refs #1202.

Committed reviews read manifests from the reviewed commit or range endpoint; dirty reviews read the working tree. Each version is labeled with its go.mod path, so root and nested modules can declare different versions. Missing or invalid version declarations contribute no fact.

The toolchain section uses the existing optional prompt sections and is trimmed after the other optional context.

@roborev-ci

roborev-ci Bot commented Oct 6, 2026

Copy link
Copy Markdown

roborev: Combined Review (bcac0ec)

Verdict: Changes require fixes for 2 findings.

Medium

  • internal/prompt/toolchain.go:20: Committed reviews read go.mod from the working tree rather than the reviewed commit or range endpoint. Older commits, other branches, or uncommitted manifest edits can therefore produce an incorrect authoritative Go version; review isolation defaults to false.

    Fix: Read go.mod using git.ReadFile at the reviewed commit or range endpoint for committed reviews, retaining filesystem reads for dirty reviews. Test differing checkout and reviewed versions.

  • internal/prompt/toolchain.go:20: Changes in nested modules always use the repository-root go.mod. When root and nested modules declare different versions, the prompt incorrectly applies the root version to nested code. The nested/go.mod test reinforces this incorrect selection.

    Fix: Resolve the governing go.mod for each changed Go file or manifest and emit module-scoped version facts, or omit the root-version assertion for nested-module files. Test differing root and nested versions.


Reviewers: 2x codex, codex (security) | Synthesis: codex, 11s | Total: 1m44s

@mariusvniekerk mariusvniekerk self-assigned this Oct 8, 2026
@mariusvniekerk
mariusvniekerk force-pushed the prompt/toolchain-version-fact branch from bcac0ec to a6a51db Compare October 8, 2026 13:49
@roborev-ci

roborev-ci Bot commented Oct 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (a6a51db)

Verdict: Changes require fixes for 2 findings.

Medium

  • internal/prompt/toolchain.go:20: Committed and range reviews read go.mod from the current checkout instead of the reviewed revision. Historical reviews, other-branch reviews, and CI prompts prebuilt from the main checkout can therefore receive an incorrect Go version and produce incorrect compatibility findings.

    Fix: Pass the reviewed commit or range endpoint to the helper and read go.mod using git.ReadFile. Reserve filesystem reads for dirty reviews and test differing checkout and reviewed versions.

  • internal/prompt/toolchain.go:20: Changes inside nested Go modules always receive the root module's declared Go version, which can suppress valid compatibility findings when the modules declare different versions.

    Fix: Resolve the governing go.mod for changed Go files and emit module-scoped version facts. Add coverage for root and nested modules declaring different versions.


Reviewers: 2x codex, codex (security) | Synthesis: codex, 12s | Total: 4m20s

@roborev-ci

roborev-ci Bot commented Oct 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (4ddbc3e)

Verdict: Changes require fixes for 2 findings.

Medium

  • internal/prompt/toolchain.go:20: Committed and range reviews read go.mod from the current checkout rather than the reviewed revision, supplying an incorrect compatibility baseline when the versions differ. Review isolation is disabled by default.

    Fix: Read go.mod at the reviewed commit or range endpoint using git.ReadFile. Reserve filesystem reads for dirty reviews and test differing checkout and reviewed versions.

  • internal/prompt/toolchain.go:20: Changes inside nested Go modules receive the root module's Go version, potentially authorizing unsupported language features and suppressing compatibility findings. Repositories containing only nested modules receive no version context.

    Fix: Resolve the governing go.mod for each changed Go file and emit module-scoped version facts. Cover modules with differing Go directives and repositories containing only nested modules.


Reviewers: 2x codex, codex (security) | Synthesis: codex, 13s | Total: 4m43s

@mariusvniekerk
mariusvniekerk force-pushed the prompt/toolchain-version-fact branch from 4ddbc3e to 8f1ccf3 Compare October 9, 2026 19:31
@roborev-ci

roborev-ci Bot commented Oct 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (8f1ccf3)

Verdict: Changes require fixes for 2 findings.

Medium

  • internal/prompt/toolchain.go:20: Committed reviews read go.mod from the working tree instead of the reviewed revision. When those versions differ, the prompt supplies incorrect authoritative context and can cause false or missed compatibility findings.

    Fix: Read go.mod from the reviewed commit or range head for committed reviews; retain filesystem reads for dirty reviews. Add coverage where the checkout and reviewed revision declare different versions.

  • internal/prompt/toolchain.go:20; internal/prompt/toolchain.go:28: Reviews of changes inside nested Go modules receive the repository-root module’s Go version, potentially overlooking features unsupported by the nested module’s declared version.

    Fix: Resolve the governing go.mod for changed Go files and scope each version to its module, or omit the blanket version statement when it does not apply to all changed modules. Test differing root and nested versions.


Reviewers: 2x codex, codex (security) | Synthesis: codex, 13s | Total: 3m58s

@mariusvniekerk
mariusvniekerk force-pushed the prompt/toolchain-version-fact branch from 8f1ccf3 to d3a2c98 Compare October 14, 2026 01:14
@roborev-ci

roborev-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (d3a2c98)

Verdict: Changes require fixes for 3 findings.

High

  • internal/prompt/golden_test.go:132: TestSinglePromptStatesGoToolchainVersion duplicates TestGoldenPrompt_SingleWithToolchain with identical repository setup and Build arguments. Its substring assertion is already covered by the golden fixture, adding git subprocesses without unique regression coverage.

    Fix: Delete TestSinglePromptStatesGoToolchainVersion and its comment; retain TestGoldenPrompt_SingleWithToolchain and its golden fixture.

Medium

  • internal/prompt/toolchain.go:20: Committed reviews read go.mod from the working checkout rather than the reviewed revision. Historical or other-branch reviews and CI prebuilt prompts can therefore assert an unrelated Go version and produce false compatibility findings.

    Fix: Read go.mod from the reviewed commit or range endpoint using git.ReadFile. Keep filesystem reads for dirty reviews and test cases where checkout and reviewed versions differ.

  • internal/prompt/toolchain.go:20: Changed Go files in nested modules receive the root module's Go version, even when their governing go.mod declares a different version.

    Fix: Resolve the governing go.mod for changed Go files and scope version facts to each module, or omit the root-module assertion for nested-module changes. Test differing root and nested versions.


Reviewers: 3x codex, codex (security) | Synthesis: codex, 12s | Total: 3m26s

subaru-ye and others added 2 commits October 14, 2026 12:26
The toolchain verification instruction asks reviewers to judge
version-gated features against the project's manifests, but it is a
passive ask: the reviewer must choose to consult go.mod and recall the
feature matrix correctly. Reviewers still flag valid recent language
features as compilation errors even when go.mod in the same diff
declares the supporting version (kenn-io#1202).

Extract the go directive from the repository's go.mod at prompt-build
time and state it as an explicit "## Toolchain" fact whenever the
change touches Go files. The section rides the existing optional
sections pipeline next to dependency metadata across single, range,
and dirty prompts, and is trimmed last when a prompt must shrink.

Refs kenn-io#1202
The coverage run returned no lines for a completed job whose log was
written immediately after datetime('now'). That value is whole seconds,
so the file modification time can still fall in the previous second.
The output endpoint then treats the new log as an older attempt.

The stale-log case already pins an older modification time. The restore
cases now pin a modification time after the attempt start.

Generated with Grok (4.7)
Co-authored-by: Grok <noreply@x.ai>
@mariusvniekerk
mariusvniekerk force-pushed the prompt/toolchain-version-fact branch from d3a2c98 to 482416a Compare October 14, 2026 12:29
@roborev-ci

roborev-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (482416a)

Verdict: Changes require fixes for 3 findings.

High

  • internal/prompt/golden_test.go:132: TestSinglePromptStatesGoToolchainVersion duplicates TestGoldenPrompt_SingleWithToolchain with identical commits and Build arguments. Its Contains assertion adds no distinct regression coverage while repeating repository setup and Git subprocesses.

    Fix: Delete TestSinglePromptStatesGoToolchainVersion and its comment; retain TestGoldenPrompt_SingleWithToolchain and its golden fixture.

Medium

  • internal/prompt/toolchain.go:20: Committed reviews read go.mod from the working tree instead of the reviewed revision. Checkout differences or uncommitted edits can supply an incorrect authoritative Go version, causing false compatibility findings or suppressing valid ones.

    Fix: Read go.mod from the reviewed commit or range endpoint using git.ReadFile. Retain filesystem reads for dirty reviews and add coverage where checkout and reviewed versions differ.

  • internal/prompt/toolchain.go:20; internal/prompt/toolchain.go:28: Changed Go files in nested modules receive the repository-root module's Go version, even when their owning module declares a different version. The existing nested/go.mod test reinforces this incorrect attribution.

    Fix: Resolve the owning go.mod for changed Go files and scope version statements by module, or omit the authoritative assertion when it does not apply. Test differing root and nested module versions.


Reviewers: 3x codex, codex (security) | Synthesis: codex, 20s | Total: 8m8s

The toolchain golden test already exercises the same repository setup and
prompt build and checks the declared Go version in its expected output.
Remove the duplicate substring test to avoid repeating Git setup without
adding regression coverage.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

This comment has been minimized.

@wesm wesm self-assigned this Oct 15, 2026
An authoritative compatibility baseline must describe the code under
review. A newer checkout or root module can otherwise hide compatibility
problems in historical commits and nested modules.

Each module needs its own declared version, including when multiple
modules appear in one review. Dirty reviews must still reflect local
manifest edits.

Addresses both toolchain findings in:
kenn-io#1332 (comment)

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

This comment has been minimized.

Non-ASCII directory names caused committed reviews to lose module context
and dirty reviews to report the root module's Go version. Compatibility
facts must describe the module that owns each changed file, regardless of
how Git quotes its name for display.

Addresses the filename finding in:
kenn-io#1332 (comment)

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Oct 16, 2026

Copy link
Copy Markdown

roborev: Combined Review (6bbe8ce)

Verdict: No findings at or above medium severity.


Reviewers: 3x codex, codex (security) | Synthesis: codex | Total: 4m24s

@wesm
wesm merged commit e3326c8 into kenn-io:main Oct 16, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants