Skip to content

Commit 12a09a8

Browse files
refactor(prompt): share common prompt build setup
1 parent 7d25301 commit 12a09a8

3 files changed

Lines changed: 69 additions & 112 deletions

File tree

internal/prompt/prompt.go

Lines changed: 67 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -91,39 +91,34 @@ func (b *Builder) Build(repoPath, gitRef string, repoID int64, contextCount int,
9191
// BuildWithAdditionalContext constructs a review prompt with an optional
9292
// caller-provided markdown context block inserted ahead of the current diff.
9393
func (b *Builder) BuildWithAdditionalContext(repoPath, gitRef string, repoID int64, contextCount int, agentName, reviewType, minSeverity, additionalContext string) (string, error) {
94-
opts := buildOpts{
94+
return b.buildWithOpts(repoPath, gitRef, repoID, contextCount, agentName, reviewType, buildOpts{
9595
additionalContext: additionalContext,
9696
minSeverity: minSeverity,
97-
}
98-
if git.IsRange(gitRef) {
99-
return b.buildRangePrompt(repoPath, gitRef, repoID, contextCount, agentName, reviewType, opts)
100-
}
101-
return b.buildSinglePrompt(repoPath, gitRef, repoID, contextCount, agentName, reviewType, opts)
97+
})
10298
}
10399

104100
// BuildWithAdditionalContextAndDiffFile constructs a review prompt with
105101
// caller-provided markdown context and an optional oversized-diff file reference.
106102
func (b *Builder) BuildWithAdditionalContextAndDiffFile(repoPath, gitRef string, repoID int64, contextCount int, agentName, reviewType, minSeverity, additionalContext, diffFilePath string) (string, error) {
107-
opts := buildOpts{
103+
return b.buildWithOpts(repoPath, gitRef, repoID, contextCount, agentName, reviewType, buildOpts{
108104
additionalContext: additionalContext,
109105
diffFilePath: diffFilePath,
110106
requireDiffFile: true,
111107
minSeverity: minSeverity,
112-
}
113-
if git.IsRange(gitRef) {
114-
return b.buildRangePrompt(repoPath, gitRef, repoID, contextCount, agentName, reviewType, opts)
115-
}
116-
return b.buildSinglePrompt(repoPath, gitRef, repoID, contextCount, agentName, reviewType, opts)
108+
})
117109
}
118110

119111
// BuildWithDiffFile constructs a review prompt where a pre-written diff file
120112
// is referenced for large diffs instead of inline content.
121113
func (b *Builder) BuildWithDiffFile(repoPath, gitRef string, repoID int64, contextCount int, agentName, reviewType, minSeverity, diffFilePath string) (string, error) {
122-
opts := buildOpts{
114+
return b.buildWithOpts(repoPath, gitRef, repoID, contextCount, agentName, reviewType, buildOpts{
123115
diffFilePath: diffFilePath,
124116
requireDiffFile: true,
125117
minSeverity: minSeverity,
126-
}
118+
})
119+
}
120+
121+
func (b *Builder) buildWithOpts(repoPath, gitRef string, repoID int64, contextCount int, agentName, reviewType string, opts buildOpts) (string, error) {
127122
if git.IsRange(gitRef) {
128123
return b.buildRangePrompt(repoPath, gitRef, repoID, contextCount, agentName, reviewType, opts)
129124
}
@@ -229,26 +224,11 @@ func (b *Builder) BuildDirtyWithSnapshot(repoPath, diff string, repoID int64, co
229224
// The diff is provided directly since it was captured at enqueue time.
230225
// reviewType selects the system prompt variant (e.g., "security"); any default alias (see config.IsDefaultReviewType) uses the standard prompt.
231226
func (b *Builder) BuildDirty(repoPath, diff string, repoID int64, contextCount int, agentName, reviewType, minSeverity string) (string, error) {
232-
// Start with system prompt for dirty changes
233-
promptType := "dirty"
234-
if !config.IsDefaultReviewType(reviewType) {
235-
promptType = reviewType
236-
}
237-
if promptType == config.ReviewTypeDesign {
238-
promptType = "design-review"
239-
}
240-
promptCap := b.resolveMaxPromptSize(repoPath)
241-
requiredPrefix := GetSystemPrompt(agentName, promptType) + "\n"
242-
if inst := config.SeverityInstruction(minSeverity); inst != "" {
243-
requiredPrefix += inst + "\n"
244-
}
245-
requiredPrefix = hardCapPrompt(requiredPrefix, promptCap)
246-
247-
optional := optionalSectionsView{}
227+
ctx := b.newPromptBuildContext(repoPath, agentName, reviewType, minSeverity, "dirty", optionalSectionsView{})
248228

249229
// Add project-specific guidelines if configured
250230
if repoCfg, err := config.LoadRepoConfig(repoPath); err == nil && repoCfg != nil {
251-
optional.ProjectGuidelines = buildProjectGuidelinesSectionView(repoCfg.ReviewGuidelines)
231+
ctx.optional.ProjectGuidelines = buildProjectGuidelinesSectionView(repoCfg.ReviewGuidelines)
252232
}
253233

254234
// Get previous reviews for context (use HEAD as reference point)
@@ -257,18 +237,18 @@ func (b *Builder) BuildDirty(repoPath, diff string, repoID int64, contextCount i
257237
if err == nil {
258238
contexts, err := b.getPreviousReviewContexts(repoPath, headSHA, contextCount)
259239
if err == nil && len(contexts) > 0 {
260-
optional.PreviousReviews = orderedPreviousReviewViews(contexts)
240+
ctx.optional.PreviousReviews = orderedPreviousReviewViews(contexts)
261241
}
262242
}
263243
}
264244

265-
bodyLimit := max(0, promptCap-len(requiredPrefix))
245+
bodyLimit := max(0, ctx.promptCap-len(ctx.requiredPrefix))
266246
inlineDiff, err := renderInlineDiff(diff)
267247
if err != nil {
268248
return "", err
269249
}
270250
view := dirtyPromptView{
271-
Optional: optional,
251+
Optional: ctx.optional,
272252
Current: dirtyChangesSectionView{
273253
Description: "The following changes have not yet been committed.",
274254
},
@@ -365,7 +345,7 @@ func (b *Builder) BuildDirty(repoPath, diff string, repoID int64, contextCount i
365345
if err != nil {
366346
return "", err
367347
}
368-
return requiredPrefix + hardCapPrompt(body, bodyLimit), nil
348+
return ctx.requiredPrefix + hardCapPrompt(body, bodyLimit), nil
369349
}
370350

371351
func isCodexReviewAgent(agentName string) bool {
@@ -409,6 +389,39 @@ type buildOpts struct {
409389
minSeverity string
410390
}
411391

392+
type promptBuildContext struct {
393+
requiredPrefix string
394+
optional optionalSectionsView
395+
promptCap int
396+
}
397+
398+
func (b *Builder) newPromptBuildContext(repoPath, agentName, reviewType, minSeverity, defaultPromptType string, optional optionalSectionsView) promptBuildContext {
399+
promptType := defaultPromptType
400+
if !config.IsDefaultReviewType(reviewType) {
401+
promptType = reviewType
402+
}
403+
if promptType == config.ReviewTypeDesign {
404+
promptType = "design-review"
405+
}
406+
promptCap := b.resolveMaxPromptSize(repoPath)
407+
requiredPrefix := GetSystemPrompt(agentName, promptType) + "\n"
408+
if inst := config.SeverityInstruction(minSeverity); inst != "" {
409+
requiredPrefix += inst + "\n"
410+
}
411+
return promptBuildContext{
412+
requiredPrefix: hardCapPrompt(requiredPrefix, promptCap),
413+
optional: optional,
414+
promptCap: promptCap,
415+
}
416+
}
417+
418+
func defaultOptionalSections(repoPath, additionalContext string) optionalSectionsView {
419+
return optionalSectionsView{
420+
ProjectGuidelines: buildProjectGuidelinesSectionView(LoadGuidelines(repoPath)),
421+
AdditionalContext: buildAdditionalContextSection(additionalContext),
422+
}
423+
}
424+
412425
func diffFileFallbackVariants(heading, filePath string) []string {
413426
if filePath == "" {
414427
return []string{heading + "\n\n(Diff too large to include inline)\n"}
@@ -686,36 +699,18 @@ func selectRichestRangePromptView(limit int, view TemplateContext, variants []di
686699

687700
// buildSinglePrompt constructs a prompt for a single commit
688701
func (b *Builder) buildSinglePrompt(repoPath, sha string, repoID int64, contextCount int, agentName, reviewType string, opts buildOpts) (string, error) {
689-
// Start with system prompt
690-
promptType := "review"
691-
if !config.IsDefaultReviewType(reviewType) {
692-
promptType = reviewType
693-
}
694-
if promptType == config.ReviewTypeDesign {
695-
promptType = "design-review"
696-
}
697-
promptCap := b.resolveMaxPromptSize(repoPath)
698-
requiredPrefix := GetSystemPrompt(agentName, promptType) + "\n"
699-
if inst := config.SeverityInstruction(opts.minSeverity); inst != "" {
700-
requiredPrefix += inst + "\n"
701-
}
702-
requiredPrefix = hardCapPrompt(requiredPrefix, promptCap)
703-
704-
optional := optionalSectionsView{
705-
ProjectGuidelines: buildProjectGuidelinesSectionView(LoadGuidelines(repoPath)),
706-
AdditionalContext: buildAdditionalContextSection(opts.additionalContext),
707-
}
702+
ctx := b.newPromptBuildContext(repoPath, agentName, reviewType, opts.minSeverity, "review", defaultOptionalSections(repoPath, opts.additionalContext))
708703

709704
// Get previous reviews if requested
710705
if contextCount > 0 && b.db != nil {
711706
contexts, err := b.getPreviousReviewContexts(repoPath, sha, contextCount)
712707
if err == nil && len(contexts) > 0 {
713-
optional.PreviousReviews = orderedPreviousReviewViews(contexts)
708+
ctx.optional.PreviousReviews = orderedPreviousReviewViews(contexts)
714709
}
715710
}
716711

717712
// Include previous review attempts for this same commit (for re-reviews)
718-
optional.PreviousAttempts = previousAttemptViewsFromContexts(b.previousAttemptContexts(sha))
713+
ctx.optional.PreviousAttempts = previousAttemptViewsFromContexts(b.previousAttemptContexts(sha))
719714

720715
// Current commit section
721716
shortSHA := git.ShortSHA(sha)
@@ -750,7 +745,7 @@ func (b *Builder) buildSinglePrompt(repoPath, sha string, repoID int64, contextC
750745
}
751746

752747
excludes := b.resolveExcludes(repoPath, reviewType)
753-
bodyLimit := max(0, promptCap-len(requiredPrefix))
748+
bodyLimit := max(0, ctx.promptCap-len(ctx.requiredPrefix))
754749
diffLimit := max(0, bodyLimit-len(currentRequired)-len(currentOverflow)-len(emptyDiffBlock))
755750
diff, truncated, err := git.GetDiffLimited(repoPath, sha, diffLimit, excludes...)
756751
if err != nil {
@@ -763,11 +758,11 @@ func (b *Builder) buildSinglePrompt(repoPath, sha string, repoID int64, contextC
763758
if opts.diffFilePath == "" && opts.requireDiffFile {
764759
return "", ErrDiffTruncatedNoFile
765760
}
766-
optionalPrefix, err := renderOptionalSectionsPrefix(optional)
761+
optionalPrefix, err := renderOptionalSectionsPrefix(ctx.optional)
767762
if err != nil {
768763
return "", err
769764
}
770-
return buildPromptPreservingCurrentSection(requiredPrefix, optionalPrefix, currentRequired, currentOverflow, promptCap, diffFileFallbackVariants("### Diff", opts.diffFilePath)...), nil
765+
return buildPromptPreservingCurrentSection(ctx.requiredPrefix, optionalPrefix, currentRequired, currentOverflow, ctx.promptCap, diffFileFallbackVariants("### Diff", opts.diffFilePath)...), nil
771766
}
772767
pathspecArgs := safeForMarkdown(git.FormatExcludeArgs(excludes))
773768
if isCodexReviewAgent(agentName) {
@@ -776,7 +771,7 @@ func (b *Builder) buildSinglePrompt(repoPath, sha string, repoID int64, contextC
776771
if err != nil {
777772
return "", err
778773
}
779-
optionalPrefix, err := renderOptionalSectionsPrefix(optional)
774+
optionalPrefix, err := renderOptionalSectionsPrefix(ctx.optional)
780775
if err != nil {
781776
return "", err
782777
}
@@ -806,52 +801,34 @@ func (b *Builder) buildSinglePrompt(repoPath, sha string, repoID int64, contextC
806801
body, err := fitSinglePromptContext(
807802
bodyLimit,
808803
templateContextFromSingleView(singlePromptView{
809-
Optional: optional,
804+
Optional: ctx.optional,
810805
Current: currentView,
811806
Diff: diffView,
812807
}),
813808
)
814809
if err != nil {
815810
return "", err
816811
}
817-
return requiredPrefix + body, nil
812+
return ctx.requiredPrefix + body, nil
818813
}
819814

820815
// buildRangePrompt constructs a prompt for a commit range
821816
func (b *Builder) buildRangePrompt(repoPath, rangeRef string, repoID int64, contextCount int, agentName, reviewType string, opts buildOpts) (string, error) {
822-
// Start with system prompt for ranges
823-
promptType := "range"
824-
if !config.IsDefaultReviewType(reviewType) {
825-
promptType = reviewType
826-
}
827-
if promptType == config.ReviewTypeDesign {
828-
promptType = "design-review"
829-
}
830-
promptCap := b.resolveMaxPromptSize(repoPath)
831-
requiredPrefix := GetSystemPrompt(agentName, promptType) + "\n"
832-
if inst := config.SeverityInstruction(opts.minSeverity); inst != "" {
833-
requiredPrefix += inst + "\n"
834-
}
835-
requiredPrefix = hardCapPrompt(requiredPrefix, promptCap)
836-
837-
optional := optionalSectionsView{
838-
ProjectGuidelines: buildProjectGuidelinesSectionView(LoadGuidelines(repoPath)),
839-
AdditionalContext: buildAdditionalContextSection(opts.additionalContext),
840-
}
817+
ctx := b.newPromptBuildContext(repoPath, agentName, reviewType, opts.minSeverity, "range", defaultOptionalSections(repoPath, opts.additionalContext))
841818

842819
// Get previous reviews from before the range start
843820
if contextCount > 0 && b.db != nil {
844821
startSHA, err := git.GetRangeStart(repoPath, rangeRef)
845822
if err == nil {
846823
contexts, err := b.getPreviousReviewContexts(repoPath, startSHA, contextCount)
847824
if err == nil && len(contexts) > 0 {
848-
optional.PreviousReviews = orderedPreviousReviewViews(contexts)
825+
ctx.optional.PreviousReviews = orderedPreviousReviewViews(contexts)
849826
}
850827
}
851828
}
852829

853830
// Include previous review attempts for this same range (for re-reviews)
854-
optional.PreviousAttempts = previousAttemptViewsFromContexts(b.previousAttemptContexts(rangeRef))
831+
ctx.optional.PreviousAttempts = previousAttemptViewsFromContexts(b.previousAttemptContexts(rangeRef))
855832

856833
// Get commits in range
857834
commits, err := git.GetRangeCommits(repoPath, rangeRef)
@@ -888,7 +865,7 @@ func (b *Builder) buildRangePrompt(repoPath, rangeRef string, repoID int64, cont
888865
}
889866

890867
excludes := b.resolveExcludes(repoPath, reviewType)
891-
bodyLimit := max(0, promptCap-len(requiredPrefix))
868+
bodyLimit := max(0, ctx.promptCap-len(ctx.requiredPrefix))
892869
diffLimit := max(0, bodyLimit-len(currentRequiredText)-len(currentOverflowText)-len(emptyDiffBlock))
893870
diff, truncated, err := git.GetRangeDiffLimited(repoPath, rangeRef, diffLimit, excludes...)
894871
if err != nil {
@@ -901,24 +878,24 @@ func (b *Builder) buildRangePrompt(repoPath, rangeRef string, repoID int64, cont
901878
if opts.diffFilePath == "" && opts.requireDiffFile {
902879
return "", ErrDiffTruncatedNoFile
903880
}
904-
optionalPrefix, err := renderOptionalSectionsPrefix(optional)
881+
optionalPrefix, err := renderOptionalSectionsPrefix(ctx.optional)
905882
if err != nil {
906883
return "", err
907884
}
908-
return buildPromptPreservingCurrentSection(requiredPrefix, optionalPrefix, currentRequiredText, currentOverflowText, promptCap, diffFileFallbackVariants("### Combined Diff", opts.diffFilePath)...), nil
885+
return buildPromptPreservingCurrentSection(ctx.requiredPrefix, optionalPrefix, currentRequiredText, currentOverflowText, ctx.promptCap, diffFileFallbackVariants("### Combined Diff", opts.diffFilePath)...), nil
909886
}
910887
pathspecArgs := safeForMarkdown(git.FormatExcludeArgs(excludes))
911888
if isCodexReviewAgent(agentName) {
912889
variants := codexRangeInspectionFallbackVariants(rangeRef, pathspecArgs)
913890
selectedCtx, err := selectRichestRangePromptView(bodyLimit, templateContextFromRangeView(rangePromptView{
914-
Optional: optional,
891+
Optional: ctx.optional,
915892
Current: currentView,
916893
}), variants)
917894
if err != nil {
918895
return "", err
919896
}
920897
if selectedCtx.Review != nil {
921-
optional = optionalSectionsView{
898+
ctx.optional = optionalSectionsView{
922899
ProjectGuidelines: buildProjectGuidelinesSectionView(selectedCtx.Review.Optional.ProjectGuidelinesBody()),
923900
AdditionalContext: selectedCtx.Review.Optional.AdditionalContext,
924901
PreviousReviews: previousReviewViewsFromTemplateContext(selectedCtx.Review.Optional.PreviousReviews),
@@ -951,15 +928,15 @@ func (b *Builder) buildRangePrompt(repoPath, rangeRef string, repoID int64, cont
951928
body, err := fitRangePromptContext(
952929
bodyLimit,
953930
templateContextFromRangeView(rangePromptView{
954-
Optional: optional,
931+
Optional: ctx.optional,
955932
Current: currentView,
956933
Diff: diffView,
957934
}),
958935
)
959936
if err != nil {
960937
return "", err
961938
}
962-
return requiredPrefix + body, nil
939+
return ctx.requiredPrefix + body, nil
963940
}
964941

965942
func buildProjectGuidelinesSectionView(guidelines string) *markdownSectionView {

internal/prompt/prompt_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,8 @@ func TestBuildWithDiffFileNonCodexUsesDiffFile(t *testing.T) {
539539
}
540540

541541
func TestBuildWithDiffFileSmallDiffInlineIgnoresFile(t *testing.T) {
542-
repoPath, sha := setupSmallDiffRepo(t)
542+
repoPath, commits := setupTestRepo(t)
543+
sha := commits[len(commits)-1]
543544

544545
b := NewBuilder(nil)
545546
diffFile := filepath.Join(repoPath, ".roborev-review-42.diff")

internal/prompt/test_helpers_test.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -97,27 +97,6 @@ func setupTestRepo(t *testing.T) (string, []string) {
9797
return r.dir, commits
9898
}
9999

100-
func setupSmallDiffRepo(t *testing.T) (string, string) {
101-
t.Helper()
102-
r := newTestRepo(t)
103-
104-
require.NoError(t, os.WriteFile(
105-
filepath.Join(r.dir, "base.txt"),
106-
[]byte("base\n"), 0o644,
107-
))
108-
r.git("add", "base.txt")
109-
r.git("commit", "-m", "initial")
110-
111-
require.NoError(t, os.WriteFile(
112-
filepath.Join(r.dir, "small.txt"),
113-
[]byte("hello world\n"), 0o644,
114-
))
115-
r.git("add", "small.txt")
116-
r.git("commit", "-m", "small change")
117-
118-
return r.dir, r.git("rev-parse", "HEAD")
119-
}
120-
121100
func setupLargeDiffRepo(t *testing.T) (string, string) {
122101
t.Helper()
123102
r := newTestRepo(t)

0 commit comments

Comments
 (0)