Skip to content

Commit e6357ab

Browse files
mariusvniekerkmariusvniekerk
committed
refactor(prompt): collapse remaining prompt context wrappers
1 parent dbe3383 commit e6357ab

3 files changed

Lines changed: 66 additions & 107 deletions

File tree

‎internal/prompt/prompt.go‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ func compareRangeMetadataLoss(a, b rangeMetadataLoss) int {
394394
}
395395
}
396396

397-
func measureOptionalSectionsLoss(original, trimmed optionalSectionsView) int {
397+
func measureOptionalSectionsLoss(original, trimmed ReviewOptionalContext) int {
398398
loss := 0
399399
if len(original.PreviousAttempts) > 0 && len(trimmed.PreviousAttempts) == 0 {
400400
loss++
@@ -411,37 +411,45 @@ func measureOptionalSectionsLoss(original, trimmed optionalSectionsView) int {
411411
return loss
412412
}
413413

414-
func measureRangeMetadataLoss(original, trimmed rangePromptView) rangeMetadataLoss {
414+
func measureRangeMetadataLoss(original, trimmed TemplateContext) rangeMetadataLoss {
415+
if original.Review == nil || trimmed.Review == nil || original.Review.Subject.Range == nil || trimmed.Review.Subject.Range == nil {
416+
return rangeMetadataLoss{}
417+
}
415418
loss := rangeMetadataLoss{
416-
RemovedEntries: len(original.Current.Entries) - len(trimmed.Current.Entries),
417-
TrimmedOptional: measureOptionalSectionsLoss(original.Optional, trimmed.Optional),
419+
RemovedEntries: len(original.Review.Subject.Range.Entries) - len(trimmed.Review.Subject.Range.Entries),
420+
TrimmedOptional: measureOptionalSectionsLoss(original.Review.Optional, trimmed.Review.Optional),
418421
}
419-
for i := range trimmed.Current.Entries {
420-
if i >= len(original.Current.Entries) {
422+
for i := range trimmed.Review.Subject.Range.Entries {
423+
if i >= len(original.Review.Subject.Range.Entries) {
421424
break
422425
}
423-
if original.Current.Entries[i].Subject != "" && trimmed.Current.Entries[i].Subject == "" {
426+
if original.Review.Subject.Range.Entries[i].Subject != "" && trimmed.Review.Subject.Range.Entries[i].Subject == "" {
424427
loss.BlankedSubject++
425428
}
426429
}
427430
return loss
428431
}
429432

430-
func selectRichestRangePromptView(limit int, view rangePromptView, variants []diffSectionView) (rangePromptView, error) {
431-
fallback := rangePromptView{Optional: view.Optional, Current: view.Current}
432-
if len(variants) > 0 {
433-
fallback.Diff = variants[len(variants)-1]
433+
func selectRichestRangePromptView(limit int, view TemplateContext, variants []diffSectionView) (TemplateContext, error) {
434+
fallback := view.Clone()
435+
if len(variants) > 0 && fallback.Review != nil {
436+
fallback.Review.Diff = DiffContext{Heading: variants[len(variants)-1].Heading, Body: variants[len(variants)-1].Body}
437+
fallback.Review.Fallback = fallbackContextFromDiffSection(variants[len(variants)-1])
434438
}
435439
var (
436-
best rangePromptView
440+
best TemplateContext
437441
bestLoss rangeMetadataLoss
438442
haveBest bool
439443
)
440444
for _, variant := range variants {
441-
candidate := rangePromptView{Optional: view.Optional, Current: view.Current, Diff: variant}
442-
trimmed, body, err := trimRangePromptView(limit, candidate)
445+
candidate := view.Clone()
446+
if candidate.Review != nil {
447+
candidate.Review.Diff = DiffContext{Heading: variant.Heading, Body: variant.Body}
448+
candidate.Review.Fallback = fallbackContextFromDiffSection(variant)
449+
}
450+
trimmed, body, err := trimRangePromptContext(limit, candidate)
443451
if err != nil {
444-
return rangePromptView{}, err
452+
return TemplateContext{}, err
445453
}
446454
fallback = trimmed
447455
if len(body) > limit {
@@ -658,16 +666,29 @@ func (b *Builder) buildRangePrompt(repoPath, rangeRef string, repoID int64, cont
658666
pathspecArgs := safeForMarkdown(git.FormatExcludeArgs(excludes))
659667
if isCodexReviewAgent(agentName) {
660668
variants := codexRangeInspectionFallbackVariants(rangeRef, pathspecArgs)
661-
selectedView, err := selectRichestRangePromptView(bodyLimit, rangePromptView{
669+
selectedCtx, err := selectRichestRangePromptView(bodyLimit, templateContextFromRangeView(rangePromptView{
662670
Optional: optional,
663671
Current: currentView,
664-
}, variants)
672+
}), variants)
665673
if err != nil {
666674
return "", err
667675
}
668-
optional = selectedView.Optional
669-
currentView = selectedView.Current
670-
diffView = selectedView.Diff
676+
if selectedCtx.Review != nil {
677+
optional = optionalSectionsView{
678+
ProjectGuidelines: buildProjectGuidelinesSectionView(selectedCtx.Review.Optional.ProjectGuidelinesBody()),
679+
AdditionalContext: selectedCtx.Review.Optional.AdditionalContext,
680+
PreviousReviews: previousReviewViewsFromTemplateContext(selectedCtx.Review.Optional.PreviousReviews),
681+
PreviousAttempts: reviewAttemptViewsFromTemplateContext(selectedCtx.Review.Optional.PreviousAttempts),
682+
}
683+
if selectedCtx.Review.Subject.Range != nil {
684+
entries := make([]commitRangeEntryView, 0, len(selectedCtx.Review.Subject.Range.Entries))
685+
for _, entry := range selectedCtx.Review.Subject.Range.Entries {
686+
entries = append(entries, commitRangeEntryView(entry))
687+
}
688+
currentView = commitRangeSectionView{Count: selectedCtx.Review.Subject.Range.Count, Entries: entries}
689+
}
690+
diffView = diffSectionView{Heading: selectedCtx.Review.Diff.Heading, Body: selectedCtx.Review.Diff.Body, Fallback: selectedCtx.Review.Fallback.Rendered()}
691+
}
671692
} else {
672693
fallback, err := renderGenericRangeFallback(renderShellCommand("git", "diff", rangeRef))
673694
if err != nil {

‎internal/prompt/prompt_body_templates.go‎

Lines changed: 11 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -10,58 +10,23 @@ import (
1010
"github.com/roborev-dev/roborev/internal/storage"
1111
)
1212

13-
type markdownSectionView struct {
14-
Heading string
15-
Body string
16-
}
13+
type markdownSectionView = MarkdownSection
1714

18-
type reviewCommentView struct {
19-
Responder string
20-
Response string
21-
}
15+
type reviewCommentView = ReviewCommentTemplateContext
2216

23-
type previousReviewView struct {
24-
Commit string
25-
Output string
26-
Comments []reviewCommentView
27-
Available bool
28-
}
17+
type previousReviewView = PreviousReviewTemplateContext
2918

30-
type reviewAttemptView struct {
31-
Label string
32-
Agent string
33-
When string
34-
Output string
35-
Comments []reviewCommentView
36-
}
19+
type reviewAttemptView = ReviewAttemptTemplateContext
3720

38-
type optionalSectionsView struct {
39-
ProjectGuidelines *markdownSectionView
40-
AdditionalContext string
41-
PreviousReviews []previousReviewView
42-
PreviousAttempts []reviewAttemptView
43-
}
21+
type optionalSectionsView = ReviewOptionalContext
4422

45-
type currentCommitSectionView struct {
46-
Commit string
47-
Subject string
48-
Author string
49-
Message string
50-
}
23+
type currentCommitSectionView = SingleSubjectContext
5124

52-
type commitRangeEntryView struct {
53-
Commit string
54-
Subject string
55-
}
25+
type commitRangeEntryView = RangeEntryContext
5626

57-
type commitRangeSectionView struct {
58-
Count int
59-
Entries []commitRangeEntryView
60-
}
27+
type commitRangeSectionView = RangeSubjectContext
6128

62-
type dirtyChangesSectionView struct {
63-
Description string
64-
}
29+
type dirtyChangesSectionView = DirtySubjectContext
6530

6631
type diffSectionView struct {
6732
Heading string
@@ -87,11 +52,7 @@ type dirtyPromptView struct {
8752
Diff diffSectionView
8853
}
8954

90-
type addressAttemptView struct {
91-
Responder string
92-
Response string
93-
When string
94-
}
55+
type addressAttemptView = AddressAttemptTemplateContext
9556

9657
type addressPromptView struct {
9758
ProjectGuidelines *markdownSectionView
@@ -107,10 +68,7 @@ type reviewAttemptContext struct {
10768
Responses []storage.Response
10869
}
10970

110-
type systemPromptView struct {
111-
NoSkillsInstruction string
112-
CurrentDate string
113-
}
71+
type systemPromptView = SystemTemplateContext
11472

11573
type inlineDiffView struct {
11674
Body string
@@ -412,31 +370,6 @@ func trimRangePromptContext(limit int, ctx TemplateContext) (TemplateContext, st
412370
return ctx, body, nil
413371
}
414372

415-
func trimRangePromptView(limit int, view rangePromptView) (rangePromptView, string, error) {
416-
trimmed, body, err := trimRangePromptContext(limit, templateContextFromRangeView(view))
417-
if err != nil {
418-
return rangePromptView{}, "", err
419-
}
420-
rangeCtx := trimmed.Review.Subject.Range
421-
if trimmed.Review == nil || rangeCtx == nil {
422-
return rangePromptView{}, body, nil
423-
}
424-
entries := make([]commitRangeEntryView, 0, len(rangeCtx.Entries))
425-
for _, entry := range rangeCtx.Entries {
426-
entries = append(entries, commitRangeEntryView(entry))
427-
}
428-
return rangePromptView{
429-
Optional: optionalSectionsView{
430-
ProjectGuidelines: buildProjectGuidelinesSectionView(trimmed.Review.Optional.ProjectGuidelinesBody()),
431-
AdditionalContext: trimmed.Review.Optional.AdditionalContext,
432-
PreviousReviews: previousReviewViewsFromTemplateContext(trimmed.Review.Optional.PreviousReviews),
433-
PreviousAttempts: reviewAttemptViewsFromTemplateContext(trimmed.Review.Optional.PreviousAttempts),
434-
},
435-
Current: commitRangeSectionView{Count: rangeCtx.Count, Entries: entries},
436-
Diff: diffSectionView{Heading: trimmed.Review.Diff.Heading, Body: trimmed.Review.Diff.Body, Fallback: trimmed.Review.Fallback.Rendered()},
437-
}, body, nil
438-
}
439-
440373
func fitDirtyPromptContext(limit int, ctx TemplateContext) (string, error) {
441374
body, err := renderDirtyPromptContext(ctx)
442375
if err != nil {

‎internal/prompt/prompt_test.go‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -781,11 +781,13 @@ func TestSelectRichestRangePromptViewPrefersRicherVariantOnEqualMetadataLoss(t *
781781
require.Greater(t, len(fullShorterBody), len(richerBody),
782782
"test setup must still require metadata trimming before the richer variant fits")
783783

784-
selected, err := selectRichestRangePromptView(len(richerBody), view, variants)
784+
selected, err := selectRichestRangePromptView(len(richerBody), templateContextFromRangeView(view), variants)
785785
require.NoError(t, err)
786+
require.NotNil(t, selected.Review)
787+
require.NotNil(t, selected.Review.Subject.Range)
786788

787-
assert.Equal(t, variants[0].Fallback, selected.Diff.Fallback)
788-
assert.Equal(t, trimmedCurrent.Entries, selected.Current.Entries)
789+
assert.Equal(t, variants[0].Fallback, selected.Review.Fallback.Rendered())
790+
assert.Equal(t, []RangeEntryContext{{Commit: "abc1234", Subject: strings.Repeat("s", 200)}, {Commit: "def5678", Subject: strings.Repeat("s", 200)}, {Commit: "ghi9012"}}, selected.Review.Subject.Range.Entries)
789791
}
790792

791793
func TestSelectRichestRangePromptViewPrefersSummaryWhenRicherNeedsMoreTrimming(t *testing.T) {
@@ -810,11 +812,13 @@ func TestSelectRichestRangePromptViewPrefersSummaryWhenRicherNeedsMoreTrimming(t
810812
require.LessOrEqual(t, len(trimmedRicherBody), len(shorterBody),
811813
"test setup must allow the richer variant only after trimming more metadata than the shorter variant needs")
812814

813-
selected, err := selectRichestRangePromptView(len(shorterBody), view, variants)
815+
selected, err := selectRichestRangePromptView(len(shorterBody), templateContextFromRangeView(view), variants)
814816
require.NoError(t, err)
817+
require.NotNil(t, selected.Review)
818+
require.NotNil(t, selected.Review.Subject.Range)
815819

816-
assert.Equal(t, variants[1].Fallback, selected.Diff.Fallback)
817-
assert.Equal(t, view.Current.Entries, selected.Current.Entries)
820+
assert.Equal(t, variants[1].Fallback, selected.Review.Fallback.Rendered())
821+
assert.Equal(t, []RangeEntryContext{{Commit: "abc1234", Subject: strings.Repeat("s", 200)}, {Commit: "def5678", Subject: strings.Repeat("s", 200)}}, selected.Review.Subject.Range.Entries)
818822
}
819823

820824
func TestSelectRichestRangePromptViewPrefersOptionalContextWhenRicherNeedsMoreTrimming(t *testing.T) {
@@ -833,11 +837,12 @@ func TestSelectRichestRangePromptViewPrefersOptionalContextWhenRicherNeedsMoreTr
833837
require.LessOrEqual(t, len(trimmedRicherBody), len(shorterBody),
834838
"test setup must allow the richer variant only after trimming more optional context than the shorter variant needs")
835839

836-
selected, err := selectRichestRangePromptView(len(shorterBody), view, variants)
840+
selected, err := selectRichestRangePromptView(len(shorterBody), templateContextFromRangeView(view), variants)
837841
require.NoError(t, err)
842+
require.NotNil(t, selected.Review)
838843

839-
assert.Equal(t, variants[1].Fallback, selected.Diff.Fallback)
840-
assert.Equal(t, view.Optional.AdditionalContext, selected.Optional.AdditionalContext)
844+
assert.Equal(t, variants[1].Fallback, selected.Review.Fallback.Rendered())
845+
assert.Equal(t, view.Optional.AdditionalContext, selected.Review.Optional.AdditionalContext)
841846
}
842847

843848
func TestBuildPromptNonCodexSmallCapStaysWithinCap(t *testing.T) {

0 commit comments

Comments
 (0)