Skip to content

refactor(prompt): consolidate template context model#630

Open
mariusvniekerk wants to merge 2 commits intogotmplfrom
gotmpl-user-facing-structs
Open

refactor(prompt): consolidate template context model#630
mariusvniekerk wants to merge 2 commits intogotmplfrom
gotmpl-user-facing-structs

Conversation

@mariusvniekerk
Copy link
Copy Markdown
Collaborator

@mariusvniekerk mariusvniekerk commented Apr 6, 2026

Summary

  • consolidate the template-facing prompt data model under a single TemplateContext root
  • route prompt rendering, fitting, and range fallback selection through the unified context model
  • collapse remaining narrow prompt wrapper/view types into thin compatibility shims over the consolidated context

Stack

Test Plan

  • go test ./...

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 6, 2026

roborev: Combined Review (e6357ab)

Summary verdict: 1 medium-severity issue found; no high or critical findings.

Medium

  • internal/prompt/template_context.go:251, internal/prompt/templates/prompt_sections.md.gotmpl:71
    The new fallback model can silently render an empty diff section. diff_block now takes the fallback path whenever .Fallback.HasContent is true, but FallbackContext.Rendered() only renders Text and Dirty.Body. If callers populate Commit, Range, or Generic, the template suppresses .Diff.Body and emits no fallback content.

    Fix: either implement rendering for all structured fallback variants in Rendered()/templates, or make HasContent only report truly renderable content until those branches are supported. Add tests covering commit/range/generic fallback rendering.


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

@mariusvniekerk mariusvniekerk marked this pull request as draft April 6, 2026 23:18
@mariusvniekerk mariusvniekerk force-pushed the gotmpl-user-facing-structs branch from e6357ab to 1281801 Compare April 7, 2026 12:32
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 7, 2026

roborev: Combined Review (1281801)

Verdict: 2 medium-severity regressions in prompt behavior should be addressed before merge.

Medium

  • internal/prompt/prompt.go:821-839, internal/prompt/templates/prompt_sections.md.gotmpl:29-43
    Range-review prompts no longer include prior per-commit review context for commits inside the reviewed range. The removed InRangeReviews population/rendering path means range reviews can resurface issues that were already found earlier in the stack or fixed by later commits.

  • internal/prompt/prompt.go:1126-1139, internal/prompt/templates/prompt_sections.md.gotmpl:135-140
    BuildAddressPrompt now merges automated fix attempts and human developer comments into one PreviousAttempts block. That removes the distinct user-comment context and can cause developer guidance to be treated like another failed attempt instead of instructions to honor while fixing.


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 (1281801)

Summary: 2 medium-severity issues found; no high or critical findings.

Medium

  • Range reviews lost in-range prior review context

    • Location: internal/prompt/prompt.go:821-839, internal/prompt/templates/prompt_sections.md.gotmpl:29-43
    • Finding: Range-review prompts no longer include prior per-commit review context for commits inside the selected range. With the old InRangeReviews population/rendering path removed, the reviewer loses the “already reviewed in this range” guidance and can resurface issues that were already identified or fixed later in the stack.
    • Suggested fix: Restore an equivalent InRangeReviews field in the consolidated context and render it from optional_sections for range prompts.
  • Developer comments are conflated with failed fix attempts

    • Location: internal/prompt/prompt.go:1126-1139, internal/prompt/templates/prompt_sections.md.gotmpl:135-140
    • Finding: BuildAddressPrompt now merges automated fix attempts and human developer comments into a single PreviousAttempts block. That removes the dedicated ## User Comments section and reframes operator guidance as just another failed attempt, which can cause the agent to underweight or misinterpret explicit developer instructions.
    • Suggested fix: Keep separate fields for tool attempts and user comments in the address-template context, or preserve the prior split behavior when building the prompt.

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

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Apr 8, 2026

roborev: Combined Review (35f7bf6)

Summary verdict: Changes introduce 2 medium-severity prompt regressions that should be addressed before merge.

Medium

  • internal/prompt/prompt.go:806-823, internal/prompt/templates/prompt_sections.md.gotmpl:43
    Range prompts no longer include the prior "Per-Commit Reviews in This Range" context. This removes guidance that helped avoid re-raising issues already covered by commit-level reviews, which is a regression in range review quality. Restore the in-range review data in the consolidated context and render the corresponding section in the range prompt template.

  • internal/prompt/prompt.go:1100-1124, internal/prompt/templates/prompt_sections.md.gotmpl:135-140
    BuildAddressPrompt now folds all prior responses into a generic "Previous Addressing Attempt" bucket, and the separate "User Comments" section was removed. That causes developer comments to be mislabeled as prior fix attempts and removes guidance on how user feedback should be used, which can degrade refine/address behavior. Keep tool attempts and user comments distinct in the structured context and rendered prompt.


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 (97ec9a7)

Verdict: this refactor introduces 3 substantive regressions in prompt safety/context handling that should be addressed before merge.

High

  • Repository-boundary restriction removed
    Location: internal/prompt/templates/codex_review.md.gotmpl:5
    The Codex review prompt no longer explicitly forbids reading files outside the repository checkout. That weakens a defense-in-depth control against prompt-injection-driven exfiltration from untrusted repo content, because malicious diffs/comments can now try to steer the agent toward local secrets or unrelated files.
    Suggested fix: Restore the repo-boundary restriction, preserve the explicit allowlist for any provided diff snapshot path, and ensure any exceptional path is validated as repo-local or an approved snapshot.

  • Address prompts no longer distinguish developer comments from prior fix attempts
    Location: internal/prompt/prompt.go:1133, internal/prompt/templates/prompt_sections.md.gotmpl:132
    The refactor merges human developer comments into ## Previous Addressing Attempts, removing their distinct framing and the instruction that these comments may identify false positives or provide explicit guidance. That changes prompt semantics and risks the agent discounting important human feedback.
    Suggested fix: Restore separate handling for user comments vs. tool-generated attempts, either by reviving the split logic or by carrying response type through the template context and rendering distinct sections.

  • Range reviews lost prior per-commit review context
    Location: internal/prompt/prompt.go:836, internal/prompt/templates/prompt_sections.md.gotmpl:23
    The InRangeReviews / Per-Commit Reviews in This Range section was removed from both prompt construction and rendering. As a result, range reviews no longer see previously generated per-commit review context, increasing the risk of re-raising already reviewed issues or missing fixes that landed later in the range.
    Suggested fix: Reintroduce InRangeReviews in the consolidated template context and restore the corresponding template section in range prompts.


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 (12a09a8)

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

All review outputs were consolidated and deduplicated. No Medium, High, or Critical issues were reported by any agent.


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 (ebdf660)

Verdict: High-severity prompt-context regressions were introduced in range review and address prompt generation.

High

  • Range reviews lost per-commit review context

    • Locations: internal/prompt/prompt.go:816-831, internal/prompt/templates/prompt_sections.md.gotmpl:29-43
    • Problem: The InRangeReviews data-loading/rendering path was removed from range prompts. Range reviews no longer include existing reviews for commits inside the reviewed range, which can cause the reviewer to miss prior findings or re-raise issues that were already reviewed or fixed later in the range.
    • Fix: Restore InRangeReviews in the range prompt context, re-add the corresponding template block, and bring back the regression test covering this behavior.
  • Address prompts no longer distinguish user comments from prior tool attempts

    • Locations: internal/prompt/prompt.go:1124-1148, internal/prompt/templates/prompt_sections.md.gotmpl:135-140, internal/prompt/templates/assembled_address.md.gotmpl:1
    • Problem: BuildAddressPrompt now merges developer comments and prior tool attempts into a single PreviousAttempts section. That changes the meaning of user-authored feedback such as “false positive” or “use a different approach” by framing it as just another attempt instead of explicit guidance the model should follow.
    • Fix: Restore separate fields/sections for tool attempts and user comments, including the dedicated user-comment heading/guidance in the templates.

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

@mariusvniekerk mariusvniekerk marked this pull request as ready for review April 10, 2026 11:03
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.

1 participant