fix: prune deleted rollouts from terminal completion log - #547
Open
elren (elrensmin) wants to merge 1 commit into
Open
fix: prune deleted rollouts from terminal completion log#547elren (elrensmin) wants to merge 1 commit into
elren (elrensmin) wants to merge 1 commit into
Conversation
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes cursor-pagination correctness for the “terminal rollouts” endpoint by ensuring that deleting a rollout also removes its ID from the in-memory completion log used to paginate terminal rollouts.
Changes:
- Update
delete_rolloutto prune deleted rollout IDs from_terminal_order. - Add regression tests covering deletion effects on
/api/rollouts/terminaltotals/cursors and the delete→re-enqueue→complete flow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| agentlightning/server/routes/rollouts.py | Prunes deleted rollout IDs from the terminal completion log to keep cursor pagination consistent. |
| tests/server/test_endpoints.py | Adds regression tests to prevent reintroduction of stale/duplicate terminal-log entries after delete and re-enqueue. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+221
to
+222
| if rollout_id in _terminal_order: | ||
| _terminal_order.remove(rollout_id) |
elren (elrensmin)
force-pushed
the
fix/terminal-order-delete-prune
branch
from
August 20, 2026 13:41
e3ad925 to
0e47a67
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
delete_rollout (agentlightning/server/routes/rollouts.py:216) pops the rollout from _rollouts and _events but never removes its id from _terminal_order. The append-only completion log backing cursor pagination.
Consequences:
Fix (root cause, not symptom)
In delete_rollout, also prune the completion log:
Removing from the list shifts subsequent indices, which is exactly what keeps the cursor consistent. This also fixes the re-enqueue case: after delete prunes the id, re-completion appends exactly once.
Tests
Added two regression tests in tests/server/test_endpoints.py:
Verification
Note on pre-existing repo-wide failures (out of scope, not introduced here)
The repo does not currently pass its own lint/format gates cleanly. These are pre-existing and unrelated to this change; I deliberately left them untouched to keep the diff minimal:
There is no CONTRIBUTING doc, no .pre-commit-config.yaml, and no CI lint/test workflow in this repo (only docs + skills deploy workflows), so these are the only gates available.