Repository navigation
Port upstream 0.67.0: retain inherited counter origin for direct Codex fork chains (stacked on #610) - #677
Conversation
… chains A fork whose parent is itself a fork with no token snapshot before the child forked now resolves to the parent's inherited totals instead of leaving the child unresolved. Ancestor resolution is depth-guarded at 64 and the Codex cache schema is bumped to 5 so stale unresolved entries are rebuilt.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Reviewed by Codex gpt-6-luna (xhigh); verified and validated by Claude Thermo-nuclear review of PR #677 against the codex-direct-fork-baselines spec (0.67.0). Backend-only; no locale, a11y or bridge changes. Findings (3):
|
|
Fixes landed at the head of this branch (3 of 3 findings fixed, none left). Changes are test fixtures and comments only; no runtime behavior change. Commands run: |
…0260923' into port/micro-0.67.0-codex-direct-fork-baselines
A fork larger than its per-refresh byte allowance restarted at byte zero on every pass and never finished. Save the fork parser state with the cache entry and continue at parsed_bytes while the parent baseline is unchanged, as upstream canResumeCodexForkAccounting does. A parse from byte zero no longer starts from the cached, already used-up inherited remainder; it replays the inherited counters itself. Only a resumed parse restores the remainder. Translate the upstream direct-fork fixtures (full event_msg rows, the per-refresh byte budget only) and assert every history byte is read once.
Lane A review: fixes at 05ebe7fReviewed the whole branch against its base ( Defects found at 8e78ad1
What changed (merge of the base at 15f1091, then two commits)
Mutation checks (each reverted afterwards): removing the resume, the parent-baseline guard, the verbatim restore of the remaining counters, or the paginated-baseline flag restore each fails at least one new test. Without the flag restore, the paginated test bills 1,866,263 input tokens instead of 19,533,671. Documented deviations (also in the PR body)
Commands and results (toolchain 1.98.0, run in the lane-a worktree through the build gate)
No frontend, locale or bridge change, and no new dependencies. Changed source files stay under 1000 lines ( |
…origin for direct Codex fork chains (stacked on nesszer#610)
Summary
Codex fork chains
root -> parent (fork) -> child (fork)no longer leave the child unresolved when the intermediate parent has no token snapshot at or before the child's fork time. The parent now resolves to the cumulative counter origin it inherited from its own parent, so the child is billed only for its own tokens. Bounded scans also finish large forks now: an unfinished fork parse continues where the previous refresh stopped.CodexLineagePlanner::parent_owner_baseline(rust/src/cost_scanner/codex/logical_target.rs) falls back to the parent's inherited totals when the parent forked at or before the child's cutoff and has no own token at or before it (no token at all, or first own token after the cutoff).CODEX_MAX_LINEAGE_DEPTH); deeper chains stay unresolved.first_token_timestampper file (persisted inCodexForkAccountingState, serde default) so a parent with tokens only after the cutoff can be recognised.CODEX_CACHE_SCHEMA_VERSION4 -> 5: the Codex cost cache is rebuilt once so stale unresolved or state-less entries are repaired. This is the local stand-in for upstream's parser revision 5.CodexForkAccountingState.resume(serde default) and continues atparsed_byteswhile its parent baseline, identity and lineage are unchanged (cost_scanner/codex/fork_resume.rs). Before this, a fork larger than its per-refresh byte allowance restarted at byte zero on every refresh and never finished.Upstream reference
v0.67.0: Codex fork snapshot resolver (forkOrigininSnapshotResolution,|inherited|dependency keys, depth guard 64) andCodexDirectForkBaselineTests.canResumeCodexForkAccountingandinitialForkAccountingState(resumed partial parses only; every other parse starts with fresh fork state).Ported / Deferred
Ported: inherited-origin baseline for empty-snapshot fork parents, transitive revalidation, depth guard 64, cache rebuild on schema bump, resume of unfinished fork parses, and tests mirroring
CodexDirectForkBaselineTestswith upstream's fixture shape (fullevent_msgtoken_count rows; the bounded case limits only the per-refresh byte budget to 512) for cold, warm, bounded, cache-schema downgrade, fresh-cache "forced rescan", changed root and cycle. The bounded case also asserts that every history byte is read exactly once.Deferred or different (kept fail-closed rather than guessed):
codex_candidate_limit = 1, a stale root whose descendants have older mtimes can be scheduled after them and keep them pending.Validation
Lane A review at 05ebe7f (toolchain 1.98.0, lane-a worktree, build gate):
cargo +1.98.0 fmt --all --check: cleancargo +1.98.0 clippy --workspace --all-targets -- -D warnings: passcargo +1.98.0 test -p codexbar --lib cost_scanner: 127 passed;--lib jsonl_scanner: 69 passedcargo +1.98.0 test -p codexbar --lib -- direct_fork fork_resume paginated copied_prefix lineage_cache: 40 passed, six runs in a rowcargo +1.98.0 test -p codexbar: 2245 passed, 0 failed, 1 ignoredcargo +1.98.0 test -p codexbar-desktop-tauri: 477 passed, 1 failed (bootstrap_payload_exposes_every_provider_variant, the known catalog-size drift Isolate bootstrap payload test from real settings #684 fixed by Make the bootstrap catalog test hermetic (#684) #711; same on the base)Review comment: #677 (comment)
Affected areas
UI proof
Not applicable (no UI, tray, settings, or float-bar change).