fix(extensions): skip AdvancedSQLiteSession usage store when the branch has no turn - #4822
Open
rioyu123 wants to merge 1 commit into
Open
fix(extensions): skip AdvancedSQLiteSession usage store when the branch has no turn#4822rioyu123 wants to merge 1 commit into
rioyu123 wants to merge 1 commit into
Conversation
…ch has no turn store_run_usage() reads the current turn with _capture_current_turn(), which returns turn 0 and a None anchor when the current branch has no turn rows. _update_turn_usage_internal only ran the removed-turn guard when an anchor was present, so a None anchor bypassed it and recorded a phantom usage row for turn 0 on an empty session: get_session_usage() reported total_turns=1 while get_items() was empty. Treat a None anchor as "no turn to attribute usage to" and skip the write. The guard stays inside _update_turn_usage_internal so the failure handling in store_run_usage is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
1 participant
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.
Summary
store_run_usage()reads the current turn with_capture_current_turn(), which returns turn 0 and aNoneanchor when the current branch has no turn rows._update_turn_usage_internalonly ran the removed-turn guard when an anchor was present, so aNoneanchor bypassed it andINSERT OR REPLACErecorded a phantom usage row for turn 0. On an empty sessionget_session_usage()reportedtotal_turns: 1whileget_items()was[].This treats a
Noneanchor as "no turn to attribute usage to" and skips the write with a debug log. The guard lives inside_update_turn_usage_internal, sostore_run_usage's error handling (and the existing test that patches that method on an empty session) is unchanged; theturn_anchordocstring documents the contract.Implementation scope. Required behavior: usage is only recorded against an existing turn on the current branch; an empty branch leaves
get_session_usage()asNoneandget_turn_usage()as[]. Compatibility: the persisted schema is unchanged, existing rows are untouched, the single caller already passes the anchor. Intentionally unsupported: attributing run usage to a session that has not stored a user turn yet (the usage is dropped, logged at debug level). Existing alternative: callstore_run_usage()afterRunner.runhas persisted the turn, which is the documented flow.Test plan
test_store_run_usage_skips_when_current_branch_has_no_turnintests/extensions/memory/test_advanced_sqlite_session.py: empty session, a branch emptied bypop_item, then a real turn (usagerequests == 2,total_turns == 1, turn numbers[1]). Fails onmain(assert {... 'requests': 1, ...} is None), passes with this change.uv run ruff format,uv run ruff check, the optional-truthiness script,uv run mypy src,uv run pyright --project pyrightconfig.json, and themake testsequivalent: 8323 passed, 81 skipped, 77 serial passed; the 19 failures are the same pre-existing Windows-only symlink and daytona-example tests as on an untouchedmaincheckout.Issue number
Closes #4821
Checks
.agents/skills/code-change-verification/scripts/run.sh(nomakeon this Windows host; I ran the equivalent Makefile commands one by one, see the test plan)maincheckout)/reviewbefore submitting this PR