Repository navigation
fix(server): preserve numeric zero data_id in terminal projection - #700
Open
KVR Vishnu (VISHNU0906) wants to merge 1 commit into
Open
KVR Vishnu (VISHNU0906) wants to merge 1 commit into
KVR Vishnu (VISHNU0906) wants to merge 1 commit into
Conversation
_data_id_of used truthiness (x or y), so data_id/instance_id of 0 were treated as missing and fell through to the fallback or "". Select the first present non-None/non-empty id instead, and cover zero and fallback cases through GET /api/rollouts/terminal. Fixes microsoft#659 Signed-off-by: KVR Vishnu <46535652+VISHNU0906@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the change preserving numeric zero IDs in terminal rollout projections.
No open issues 🎉
What changed in this PR
Fixes #659 by preserving numeric zero IDs in the terminal rollout projection without changing stored rollout input.
Changes:
- Selects
data_idbeforeinstance_id, falling back only for missing, null, or empty-string values. - Adds parameterized endpoint regressions for zero IDs, ordinary IDs, and fallback behavior.
| File | Description |
|---|---|
tests/server/test_endpoints.py |
Adds a rollout lifecycle helper and nine terminal projection test cases. |
agentlightning/server/routes/rollouts.py |
Replaces truthiness-based ID selection with explicit presence checks that preserve zero. |
Review effort: Balanced
This branch has not been deployed
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.
Fixes #659
CC Quanxin Liu (@notoufa)
Problem
_data_id_ofselected IDs with truthiness (data_id or instance_id or ""), so numeric0was treated as missing.GET /api/rollouts/terminalthen returned""or aninstance_idfallback instead of"0".Change
Pick the first present
data_id/instance_idthat is notNoneor"", then stringify. Explicit0/"0"are kept; missing / null / empty still fall back.Tests
Endpoint regressions through enqueue → running → succeeded →
GET /api/rollouts/terminalfor:data_id=0withinstance_idfallbackinstance_id=0onlydata_id)""Mutation-check: reverting to the
orchain fails the three zero cases; controls still pass.Verification
ruff check/ruff format --checkon touched filespyrighton touched filespytest -v tests→ 250 passed, 4 skippedI used AI assistance for parts of this change and tested it locally against the full test suite.