Skip to content

refactor(prompt): make prompt construction template-driven#625

Merged
wesm merged 2 commits intomainfrom
gotmpl
Apr 17, 2026
Merged

refactor(prompt): make prompt construction template-driven#625
wesm merged 2 commits intomainfrom
gotmpl

Conversation

@mariusvniekerk
Copy link
Copy Markdown
Collaborator

@mariusvniekerk mariusvniekerk commented Apr 5, 2026

Why

Prompt construction in internal/prompt had accumulated too much behavior in Go-side string assembly, which made the actual prompt structure hard to reason about, easy to regress during cleanup, and split across multiple sources of truth. This refactor moves the prompt format itself into templates so the rendered prompt shape is defined where it is authored, while preserving existing review behavior and fixing the regressions uncovered during the migration and subsequent roborev reviews.

Summary

  • make prompt construction template-driven end to end by moving assembled prompt bodies, prompt sections, and agent-specific system prompts onto embedded .md.gotmpl templates backed by typed view data
  • preserve existing prompt behavior while fixing cleanup regressions in dirty and range prompt fitting, including truncated diff fallbacks, optional-context retention, and fallback selection quality
  • resolve the accumulated roborev findings on gotmpl so the branch ends with no open review jobs

Test Plan

  • go fmt ./...
  • go vet ./...
  • go test ./...

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 5, 2026

roborev: Combined Review (9e36436)

Verdict: No medium-or-higher issues found across the submitted reviews.

The reviewed changes appear clean. Both substantive review outputs reported no issues, and the security review explicitly found no externally exploitable concerns in the modified paths under this repo’s local-only trust model.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 7, 2026

roborev: Combined Review (04449e4)

Summary verdict: Changes are mostly sound, but there is one medium-severity regression in range prompt trimming that should be fixed before merge.

Medium

  • internal/prompt/prompt_body_templates.go:287
    trimOptionalSections no longer considers InRangeReviews, even though buildRangePrompt now stores per-commit reviews in optional.InRangeReviews. When a range prompt exceeds budget, the fitter can now strip subjects and current range entries before dropping this optional context, which regresses prior behavior where the optional block was trimmed first.
    Fix: include InRangeReviews in trimOptionalSections, trim it before mutating Current range metadata, and add a regression test for oversized range prompts with populated per-commit reviews.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 9, 2026

roborev: Combined Review (21a38f8)

Verdict: No Medium, High, or Critical issues found.

All review outputs are clean. I did not find any deduplicated findings at Medium severity or above to include in this PR comment.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 9, 2026

roborev: Combined Review (4bd884c)

No medium-or-higher issues found; the reviewed changes look clean.

All provided reviews reported no medium, high, or critical findings.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 9, 2026

roborev: Combined Review (488ff49)

Verdict: No medium-or-higher findings; the reviewed changes look clean.

No Medium, High, or Critical issues were identified across the submitted review outputs.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@mariusvniekerk mariusvniekerk marked this pull request as ready for review April 10, 2026 11:04
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 17, 2026

roborev: Combined Review (59a0b2c)

Verdict: changes are not ready to merge due to two high-severity prompt-regression risks.

High

  • internal/prompt/templates/default_address.md.gotmpl:27
    Appending noSkillsInstruction to the address/refine prompt appears to change the agent’s role from “apply fixes and report changes” to “perform this review” and “return only the final review content.” That conflicts with the address workflow, which is expected to modify files, run verification, and return a Changes: summary. This can cause refine/fix agents to act like reviewers instead of actually applying fixes.
    Suggested fix: remove noSkillsInstruction from address prompts, or replace it with an address-specific no-delegation block that forbids delegation without redefining the task as a review.

  • internal/prompt/prompt_body_templates.go:288
    trimOptionalSections does not trim InRangeReviews, even though range prompt construction now stores prior per-commit review history there. In large range prompts, that stale context can survive prompt fitting while newer, more relevant range metadata or diff content gets trimmed or hard-capped instead.
    Suggested fix: make InRangeReviews trimmable in trimOptionalSections before current-range metadata/diff, and add a regression test covering a large range prompt with populated InRangeReviews.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Satisfies testifylint require-error: error assertions followed by
dependent assertions must halt on failure to avoid cascading errors.
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 17, 2026

roborev: Combined Review (ed3266b)

Verdict: 2 findings require attention before merge.

High

  1. Range prompt trimming can discard the actual diff before older review context
    • Location: internal/prompt/prompt_body_templates.go:224-299, internal/prompt/prompt.go:643
    • fitRangePrompt trims optional sections when the range prompt is oversized, but trimOptionalSections does not trim InRangeReviews. In repos with many per-commit reviews, the fitter can start dropping current commit entries and eventually hard-cap the prompt while leaving ## Per-Commit Reviews in This Range intact. That can truncate or remove the combined diff the reviewer is supposed to inspect.
    • Suggested fix: make InRangeReviews trimmable optional context, preferably before sacrificing current range metadata or the diff section, and add a test covering an oversized range prompt populated with InRangeReviews.

Medium

  1. Address prompt reuses review-specific no-delegation instruction
    • Location: internal/prompt/templates/default_address.md.gotmpl:27, internal/prompt/prompt.go:30-35
    • The default address prompt now appends noSkillsInstruction, but that instruction is written for review flows: it says to “perform this review directly,” references “analyzing the diff,” and says “Return only the final review content.” That conflicts with the address workflow, which is supposed to modify code, run build/tests, and return a Changes: summary. This can steer fix/refine agents toward review-shaped output instead of making changes.
    • Suggested fix: use a separate no-delegation instruction for address prompts that forbids delegation without telling the agent to perform a review or emit review-style output.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@wesm wesm merged commit f98fd29 into main Apr 17, 2026
8 checks passed
@wesm wesm deleted the gotmpl branch April 17, 2026 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants