Skip to content

fix(pi): retain compaction helper references and usage - #5436

Open
giuliolovisotto wants to merge 4 commits into
glovisotto/compaction-overflow-fixturefrom
glovisotto/pi-compaction-evidence
Open

giuliolovisotto wants to merge 4 commits into
glovisotto/compaction-overflow-fixturefrom
glovisotto/pi-compaction-evidence

Conversation

@giuliolovisotto

@giuliolovisotto giuliolovisotto commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Pi's compaction summaries do not emit ordinary message_end events. Gym therefore omitted their usage and helper references, and the collector counted the retained helper as a policy turn.

Observe the pinned Pi 0.80.2 session's existing compaction boundaries and stream results. Publish each helper's native response ID and usage, associate it with the compaction operation, and retain it under the invocation. Helper text stays out of the task answer. Completed helper evidence survives failed or interrupted compaction; triggering, summarization and retry remain Pi's own behavior.

Compaction helper capture is provided through agent sessions. Local CLI capture is a separate follow-up.

This is the second PR in the separate Pi follow-up stack, based on #5435; no separate GitHub issue is needed. Preserve the recovered-error finality behavior from #4739. The observer uses Pi's private AgentSession._emit boundary because extension hooks do not provide a compaction failure/end boundary; runtime upgrades must revalidate that integration. Unsupported observer internals do not prevent Pi from starting; missing helper references are reported as evidence gaps.

Validation on 2026-11-06, with the assembled Pi stack rebased onto main b4dc559825a951171c0218623f1c44243ceb7843:

  • Shared conformance runner/checker and Pi suites: 872 passed, 14 skipped.
  • pre-commit run --all-files: passed.
  • Fresh pinned Pi 0.80.2 run: all nine scenarios pass with P1 gating. The compaction rollout retains 17 model calls, 15 policy turns, one referenced helper and 190,544 synthetic fixture tokens. Summary reuse, retained helper usage and helper exclusion from turns pass.
  • Range-diff confirms this PR's patches are unchanged by the rebase. Commits retain DCO sign-offs and SSH signatures.

Rollout evidence uses the real pinned runtime and shell execution with a controlled local provider. Completed compactions with missing helper references report an evidence gap. Regression tests cover cached-token contributions from helper calls, including zero and missing cache data. Existing session configuration and scoring policy are unchanged.

@giuliolovisotto giuliolovisotto added bug Something isn't working area:agent Agent harnesses and Responses API agent behavior needs-review PR is ready for code review and waiting on a reviewer labels Nov 2, 2026
@nemo-automation-bot
nemo-automation-bot Bot requested a review from a team November 2, 2026 10:15

@svcnvidia-nemo-ci svcnvidia-nemo-ci left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHIP

LGTM — no concerns found.

I reviewed all four changed files and traced the observer and sandbox launch, usage aggregation, recovered-error finality, and collector helper exclusion. I found no qualifying defects in the changed paths or their directly affected contracts.

This was static review; I didn't execute tests or independently verify the reported runs. Pi's private implementation isn't included in the snapshot, so I assessed that integration using the adapter and its existing pinned-CLI regression tests.

Note: formal approval is not available for this request, so this review is posted as a comment only.

Review generated by codex. This was a full review. Request a full review with /review reset=true.

@giuliolovisotto
giuliolovisotto force-pushed the glovisotto/pi-compaction-evidence branch from 42884bb to 1a1f17c Compare November 2, 2026 16:48
@giuliolovisotto
giuliolovisotto force-pushed the glovisotto/pi-compaction-evidence branch from 1a1f17c to 55249fe Compare November 2, 2026 17:00
@giuliolovisotto
giuliolovisotto force-pushed the glovisotto/pi-compaction-evidence branch from 55249fe to dee61f6 Compare November 3, 2026 10:08
@giuliolovisotto
giuliolovisotto force-pushed the glovisotto/pi-compaction-evidence branch from dee61f6 to 3d40e96 Compare November 3, 2026 10:13
@giuliolovisotto
giuliolovisotto force-pushed the glovisotto/pi-compaction-evidence branch from 3d40e96 to ccc2acb Compare November 3, 2026 10:28
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>
Signed-off-by: Giulio Lovisotto <glovisotto@nvidia.com>

This branch was previously deployed

1 inactive deployment
public — f6bc9506 Deployed Nov 6, 2026 by copy-pr-bot[bot] via release / finalize / notify #10636
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:agent Agent harnesses and Responses API agent behavior bug Something isn't working needs-review PR is ready for code review and waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants