Pular para conteúdo

AUR-578: the review prompt is a template with budgeted slots, sent as messages

Measurement (before the change, base e6b708fe)

  • internal/prompt/templates/review.md received {{.Metrics}}/{{.CIContext}} while buildUserContent wrote ## Change Summary, ## Existing CI Context and ## Code Changes in Go; ## PR history, ## Codebase context, ## Review memory (builder.go), ## Review Coverage (coverage.go) and ## Repository context (config/provider.go) were Go string literals.
  • config.contextInjectingProvider appended the context block after the budgeted prompt (prompt+"\n\n"+block), outside TrimToFit/fixedOverhead, and its type assertion hid optional provider interfaces.
  • Reviewer.GenerateReviewWithContext (177 lines) sent System+"\n\n"+User as one string through a concrete *llm.Orchestrator; MaxTokens: 0 disabled the prompt budget by default.
  • DefaultRuleCatalog was a hand-written mirror of review/rules/*.yml; MaxRuleCatalogTokens = 512 was a Go constant.
  • ReviewIssue had no origin nor model assessment.
  • formatLanguages iterated a map: the ## Languages section of the system prompt came out in random order (measured: two runs of the same golden test differed at byte 1900). It is now sorted; this is the only byte change to an existing section, and it replaces a nondeterministic order with a fixed one.

What changed

  • internal/prompt/templates/review.md: the main body is unchanged (system message). Named {{define}} slots now hold every user-message section: user_header, pr_history, codebase_context, review_memory, deterministic_evidence + evidence_item, available_tools + tool_item, coverage, repository_context, repository_context_slot.
  • internal/prompt/slots.go, sections.go, evidence.go: slot rendering; BuildOptions gains Evidence, Tools, RepositoryContext. Each is counted in fixedOverhead (inside the budget). Evidence and tools admit whole items under their ceiling and render - N omitidos pelo orçamento desta seção. Empty input renders nothing (cmd bytes unchanged until AUR-579).
  • internal/prompt/templates/limits.yml + limits.go: default ceilings (prompt 120000, rule catalog 512, evidence 4096, tools 1024); MaxRuleCatalogTokens derives from it; PromptBuilder.SetSlotLimits.
  • budgeting.go: when every segment fits, TrimToFit keeps the diff order (a ceiling that trims nothing must not reorder hunks; the priority sort put f:10 before f:2). TestTrimToFit_Deterministic now pins that order.
  • internal/review/rules/catalog.go: leaf package embedding the rule YAMLs; prompt.DefaultRuleCatalog derives from it.
  • internal/llm/messages.go: MessageCompleter capability, FlattenMessages, SystemUserMessages, Unwrapper and As[T] (capability lookup through pass-through decorators); ResolveModelKey and AsToolCaller use As. Orchestrator.CompleteMessages sends messages to a capable provider and the flattened text otherwise (estimate always on the flattened text). litellm.Provider.CompleteMessages sends system and user separately.
  • internal/review: Reviewer depends on Completer; GenerateReviewWithContext is four stages (preparePrompt, complete, parse, applyGates, plus annotateResult), each under 70 lines. ReviewContext gains RepositoryContext, Evidence, Tools; every evidence/tool field is redacted. Config.PromptTokenBudget defaults from limits.yml and never caps the reply. PromptDigest and RequestCacheKey.
  • internal/review/cache/request_key.go: RequestKey(prompt, evidence, tool-results digests) and DigestOf.
  • pkg/types: ReviewIssue.Origin, ReviewIssue.Assessment (EvidenceAssessment, statuses confirmed|disputed|needs_context). The parser clears a model's origin and drops an assessment with an unknown status or no evidence id; the reviewer redacts the assessment.
  • internal/config/provider.go: the block is rendered by prompt.RenderRepositoryContext (same bytes, title in the template). wrap.go: contextInjectingProvider is documented as the compatibility path and deliberately does not implement Unwrapper.
  • Dead code removed: BuildReviewPrompt, buildReviewPromptFallback (iso_scores fallback), TruncatePrompt, formatDiffContent and their tests (builder_test.go). Zero production callers (grep over cmd, internal, tests).

Kept on purpose

  • cache.PromptVersion: no production use, but tests/unit/AUR-441.go (outside this card's paths) still references it.
  • TokenBudget.truncateSegment: tests/acceptance/AUR-475.sh MUT-002 splices it.
  • contextInjectingProvider/WrapProvider: cmd still calls them.

For AUR-579 (cmd/aurumcode, not in this card's paths)

  1. Replace config.WrapProviderWithWarnings(...) by config.BuildContextBlockWithWarnings(...) and pass the block as review.ReviewContext.RepositoryContext; send through the unwrapped provider. TestRepositoryContextSlotMatchesLegacyDecoratorBytes proves the bytes a single-string provider receives are identical.
  2. Replace the positional reviewContextCacheKey (review_cache.go:149, review_pr_gate.go:47) by Reviewer.RequestCacheKey(diff, ctx, toolResults) / cache.RequestKey.
  3. profileProvider (profiles.go) and fixedModelProvider (cost.go): a pass-through decorator implements llm.Unwrapper; one that changes the request implements the capability it changes explicitly.
  4. Wire deterministic evidence (SAST, security pass) into ReviewContext.Evidence, setting ReviewIssue.Origin for engine findings.
  5. The response-format text says "Use exactly the fields shown below" while the evidence slot asks for assessment; reconcile both when evidence is wired in. An issue that only carries a disputed/needs_context assessment does not survive the precision and rule gates (and its discard withholds the summary); evidence_id is not yet checked against the evidence offered.

AC-001 scans every Go file of cmd, internal and pkg, except the three writers of human Markdown reports (cmd/aurumcode/pr.go, internal/render/summary.go, internal/changelog/render.go).

Behavior visible to cmd (deliberate, required by the card)

  • The default prompt ceiling (120000 tokens, limits.yml) is now active: a diff beyond it is trimmed by whole hunks and declared in the coverage section; fixed content beyond it is a loud error instead of a sent prompt.
  • FixedContentDigest moved (new slot sentinels): existing review caches are invalidated once.
  • ## Languages is rendered sorted (it was random).
  • An unwrapped litellm provider now receives a separate system message; when cmd wraps it (repository context, profiles, fixed model) it still receives the flattened text, until AUR-579.

Legacy mutation anchors

The reviewer stages stay in internal/review/reviewer.go and keep the lines older scripts splice (diff = redactDiff(r.filter, diff) for AUR-432, \treturn result, nil for AUR-430, withholdSummaryWhenFiltered(result.Summary, discardedByPipeline) for AUR-517).

Rerun after the change: AUR-517.sh all pass, AUR-477.sh all pass. AUR-430/432/434 (missing internal/documentation/extractors) and AUR-459/461/467/475 AC-001 (enumerated copies without internal/grammar) fail identically on the base commit e6b708fe: they were already stale, not broken by this card.

Re-anchored (card v2 put tests/acceptance/AUR-543.sh in paths): AC-001-MUT-002 spliced result.WriteString("## Code Changes\n\n") in builder.go, a Go title this card removes (AC-001). The mutation now edits the same user-half title where it lives, the user_header slot of internal/prompt/templates/review.md (## Code Changes -> ## AUR543 Mutated User-Half Header; the system half spells ## Code changes, so the anchor is unique). Same semantics, behavioral RED:

aur543_test.go:177: B1: fixed content hashed by FixedContentDigest is missing literal "## Code Changes" -- editing it would never change the cache key
--- FAIL: TestAUR543B1FixedContentCoversUserHalfAndChangeScope (0.02s)
--- PASS: TestAUR543B1DigestIsHashOfFixedContent (0.00s)
AUR-543/AC-001-MUT-002/pass (mutation produced RED, digest link verified)

bash tests/acceptance/AUR-543.sh all (nominal + all three mutations) ends with AUR-543/all/pass.

Proof (shared container and sealed profile)

Item Command Last line
AC-001 tests/acceptance/AUR-578.sh AC-001 (go/ast + grep over prompt/review/llm/config) AUR-578/AC-001/pass
AC-002 ... AC-002 (TestEvidenceSlotRendersRedactsAndDeclaresOmissions) AUR-578/AC-002/pass
AC-003 ... AC-003 (system/user messages; estimated_tokens equals the estimate of what was sent; digest stable, moved by evidence) AUR-578/AC-003/pass
AC-004 ... AC-004 (evidence omitted by the ceiling still changes the key) AUR-578/AC-004/pass
AC-005 ... AC-005 AUR-578/AC-005/pass
AC-006 ... AC-006 (goldens captured at 943ea197 before any template change; slot path == legacy decorator bytes; go test ./cmd/aurumcode/...) AUR-578/AC-006/pass
MUT-001 header back in config/provider.go + block appended after BuildPrompt AC-001: section title in Go code ("## Repository context ..."); AC-003: estimated_tokens = 4317 but the sent prompt measures 4356
MUT-002 evidence digest removed from RequestKey evidence omitted from the prompt but different shared a cache key
all tests/acceptance/AUR-578.sh all AUR-578/all/pass
sealed oci-run --profile go-unit-offline-v1 --card AUR-578 exit_code: 0, stdout AUR-578/all/pass

Suite: go vet ./... clean; go test -count=1 -p 1 ./... all ok. Regression: AUR-565.sh all pass, AUR-562.sh all pass, AUR-574.sh all pass.

AC-006 skips four cmd tests in the copy (TestAUR499ActionOutput, TestAUR555*): they read action.yml and .github/workflows/review.yml, which are not inputs of this card and are not materialized in the sealed copy; they pass in the full suite above.