Skip to content

ce-babysit-pr review-gate evals test the reasoning but not the lookup #1618

Description

@tmchow

The gap

ce-babysit-pr's readiness decision now rests on a judgment the agent makes from live GitHub state: whether a review is still on its way. skills/ce-babysit-pr/references/settle.md tells it to make one lookup across four surfaces — reactions on the PR body, top-level comments, check runs on the current head, and reviews against it — and reason from what comes back.

Five eval cells cover that reasoning (tests/skill-eval-cell/catalog.ts, ids starting ce-babysit-pr/). Every one of them hands the agent a prepared description of what the lookup would have returned and forbids gh. They are read_only: true by design, which is what makes them cheap and deterministic.

So the cells validate the semantic classification — a terminal check that accounts for the announced review clears the wait, a timed-out one does not read as approval, an unrelated check is not the review finishing, a silent reviewer of an earlier head still waits. They validate none of the acquisition:

  • Head attribution. Reactions and top-level comments are PR-scoped and carry no commit identity; check runs and reviews are commit-scoped. Nothing states how a PR-scoped announcement is bound to the current head.
  • Identity correlation. Bot logins differ across APIs — REST returns cursor[bot] and chatgpt-codex-connector[bot], GraphQL returns cursor and chatgpt-codex-connector. A lookup that assumes one shape silently misses reviewers.
  • Pagination. No stated behavior for a PR with more reactions, comments, or check runs than one page.
  • Probe failure. No stated behavior when part of the lookup fails, which is the case most likely to produce a false ready.

Why it matters

This is the seam most likely to fail in production, and it is the one place the current tests say nothing. Two independent cross-model reviews of the change raised it separately, which is the reason it is filed rather than argued.

Direction, not a prescription

A cell that permits gh against a real fixture PR would exercise acquisition, but it trades determinism for realism and needs a stable subject. An alternative is to keep the reasoning cells as they are and add deterministic coverage of the correlation rules alone — the [bot] suffix variance and the head-attribution rule are mechanical and belong in bun test rather than an eval.

Worth deciding which before adding either.

Context: #1611.

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

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions