Skip to content

bug: Aggregate a full code generation without the previous review #33

Description

@idy

Background

.github/scripts/pr-review/run.mjs aggregates a code generation with an
aggregate-input.json that carries previous_code_review whenever the Codex
session is resumable, and the aggregation prompt asks the model to "preserve
still-applicable previous findings for the current complete PR state, and
remove findings demonstrably fixed by the incremental diff". Neither the input
nor the prompt distinguishes a full generation from an incremental one.

A full generation is produced when no completed generation can be resumed:
the first review, and, since #32, every time the merge base moves because the
base branch was merged into the pull request. In that case the previous code
review described a different range. Its findings and plan-conformance blockers
are still handed to the aggregation as candidates to preserve, and the prompt
frames removal as something the "incremental diff" must demonstrate, which a
full re-review cannot.

Observed on GizClaw/gizclaw#1160: after a session had once reviewed a range
that included merged base-branch commits, every later run kept the
plan-conformance blocker naming those commits as "unrelated implementation",
including runs whose diff no longer contained them. The evidence chain had to
be discarded by deleting the stored session artifact before a clean run could
pass.

Goal

  • A full generation aggregates only its own chunk reviews. The previous code
    review is not supplied and the prompt says no previous review is carried
    over.
  • An incremental generation keeps today's behavior: the previous code review
    is supplied and still-applicable findings are preserved.

Non-goals

  • Do not change evidence reuse. An unchanged head with an unchanged diff still
    reports reused with zero model tokens.
  • Do not change the PR or Issue stages, chunking, or the session model.

Code Changes Tree

.github/
└── scripts/
    └── pr-review/
        ├── run.mjs                # supply previous_code_review only to incremental generations; full-mode aggregation prompt
        └── test.mjs               # incremental aggregation still receives the previous review; a full generation after merging a moved base receives none
README.md                          # state that a full generation aggregates only its own chunk reviews

Design

In run.mjs, aggregateInput.previous_code_review becomes
resumableSession && generation.mode === "incremental" ? previousCode?.result ?? null : null.
The aggregation prompt keeps the existing preservation sentence for
incremental generations and, for any other mode, states that the generation
reviewed the complete current pull-request diff from its merge base, that only
findings and blockers supported by these chunk reviews may be reported, and
that no previous code review is carried over. Chunk turns and the reuse check
are unchanged.

Test And Acceptance Criteria

Acceptance Criteria

  • In the existing resumed incremental scenario, aggregate-input.json has
    generation.mode incremental and a non-null previous_code_review.
  • After merging a moved base branch into the pull request and pushing further
    pull-request work, the prepared generation is full, its listing contains
    only pull-request files, and its aggregate-input.json has
    previous_code_review null even though the session is resumed and
    previous code evidence exists.
  • The full-generation assertion fails against the previous run.mjs.

Validation

  • node .github/scripts/pr-review/test.mjs
  • node .github/scripts/issue-review/test.mjs
  • node .github/scripts/pr-readiness/test.mjs
  • node .github/scripts/review-request/test.mjs

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions