Skip to content

Port upstream 0.67.0: retain inherited counter origin for direct Codex fork chains (stacked on #610) - #677

Closed
Finesssee wants to merge 5 commits into
codex/integrate-reviewed-ports-20260923from
port/micro-0.67.0-codex-direct-fork-baselines
Closed

Finesssee wants to merge 5 commits into
codex/integrate-reviewed-ports-20260923from
port/micro-0.67.0-codex-direct-fork-baselines

Conversation

@Finesssee

@Finesssee Finesssee commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

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).
  • Every ancestor on the way up is revalidated (baseline equality, file identity, mtime, size, parsed bytes), so a changed root invalidates descendants that inherited through an empty parent.
  • Ancestor resolution is depth-guarded at 64 (CODEX_MAX_LINEAGE_DEPTH); deeper chains stay unresolved.
  • The parser records first_token_timestamp per file (persisted in CodexForkAccountingState, serde default) so a parent with tokens only after the cutoff can be recognised.
  • CODEX_CACHE_SCHEMA_VERSION 4 -> 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.
  • Fork resume (Lane A review): an unfinished parent-baseline fork parse saves its parser state in CodexForkAccountingState.resume (serde default) and continues at parsed_bytes while 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.
  • Grown-fork fix (Lane A review): a fork parsed again from byte zero starts from the full inherited baseline instead of the cached, already used-up remainder, which billed inherited usage as the fork's own (root 1000, child last-only 600, 20, then 500 appended: 740 billed where a cold scan bills 120). Only a resumed parse restores the remainder.

Upstream reference

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 CodexDirectForkBaselineTests with upstream's fixture shape (full event_msg token_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):

  • A root parent with no snapshot at the cutoff stays unresolved (upstream resolves it to a nil baseline).
  • A parent with tokens on both sides of the cutoff stays unresolved; the local cache keeps only first and last token timestamps, not every event.
  • Per-file parser revision 5 and dependency keys: replaced by the cache-wide schema bump and baseline-equality ancestor validation. A resumed fork compares its saved parent baseline with the current one instead of a dependency key.
  • Changed-root case: upstream expects the child to rebill as 10 after the root is rewritten to 1010; the local raw-cumulative parser sees the child's replayed total (1000) below the new baseline and fails closed (unresolved) instead. The test asserts that safe outcome, not upstream's number.
  • Subagent forks in parent-baseline mode resume here; upstream resumes subagent threads only through its buffered-line path. This port restores the full parser state, and a lineage or identity change still forces a parse from byte zero.
  • Pre-existing, not changed here: with codex_candidate_limit = 1, a stale root whose descendants have older mtimes can be scheduled after them and keep them pending.
  • docs/codex.md and the upstream spec doc have no local counterpart; CHANGELOG entries were added instead.

Validation

Lane A review at 05ebe7f (toolchain 1.98.0, lane-a worktree, build gate):

  • cargo +1.98.0 fmt --all --check: clean
  • cargo +1.98.0 clippy --workspace --all-targets -- -D warnings: pass
  • cargo +1.98.0 test -p codexbar --lib cost_scanner: 127 passed; --lib jsonl_scanner: 69 passed
  • cargo +1.98.0 test -p codexbar --lib -- direct_fork fork_resume paginated copied_prefix lineage_cache: 40 passed, six runs in a row
  • cargo +1.98.0 test -p codexbar: 2245 passed, 0 failed, 1 ignored
  • cargo +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)
  • Mutation checks: removing the resume, the parent-baseline guard, the verbatim restore of the remaining counters, or the paginated-baseline flag restore each fails a new test.
  • Before the resolver change the chain and changed-root tests failed with "bounded Codex scan never completed"; with upstream's fixture shape the bounded chain also never completed before the fork resume.

Review comment: #677 (comment)

Affected areas

  • Rust backend (cost scanner, Codex JSONL parser and cache)
  • Providers, CLI, Tauri shell, frontend, settings, tray, float bar
  • Files: rust/src/cost_scanner/codex/logical_target.rs, rust/src/cost_scanner/codex.rs, rust/src/cost_scanner/codex/fork_resume.rs (new), rust/src/core/jsonl_scanner.rs, rust/src/core/jsonl_scanner/codex.rs, rust/src/core/jsonl_scanner/codex/parser.rs, rust/src/cost_scanner/tests.rs (mod lines), rust/src/cost_scanner/tests/direct_fork.rs (new), rust/src/cost_scanner/tests/fork_resume.rs (new), rust/src/cost_scanner/tests/paginated.rs, CHANGELOG.md. No changed source file crosses 1000 lines (tests.rs was already over and only gains mod declarations).

UI proof

Not applicable (no UI, tray, settings, or float-bar change).

… 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.
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 44ab412c-754e-478e-b4e7-b12f5df8176b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

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):

  • P2 cost_scanner/tests/direct_fork.rs:197 - t=0 stood for "empty parent", so a parent snapshot at t=0 was untested. Fixed: fixture now covers an empty parent and real parent events at t=0, 3 and 8.
  • P2 cost_scanner/tests/direct_fork.rs:132 - bounded mode only limited candidates and never set the spec's 512-byte limits. Fixed: both byte caps applied, full chain admitted, and the test asserts the scan needs multiple refreshes.
  • P3 core/jsonl_scanner.rs:399 - doc comments described the timestamp as the fork's first "own" event and as always from a full parse. Fixed: comments now describe the recorded first event and resumed-parse behavior.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

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: cargo +1.98.0 fmt --all --check; cargo +1.98.0 clippy --all-targets -- -D warnings on both manifests (clean); cargo +1.98.0 test --manifest-path rust/Cargo.toml direct_fork (5 passed) and cost_scanner (123 passed).

…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.
@Finesssee

Copy link
Copy Markdown
Collaborator Author

Lane A review: fixes at 05ebe7f

Reviewed the whole branch against its base (codex/integrate-reviewed-ports-20260923 at 15f1091) and against upstream v0.67.0 (steipete#3524, commit d834114, tag-pinned sources and the PR diff).

Defects found at 8e78ad1

  • Bounded fork parses never resumed. A parent-baseline fork parse always started at byte zero, so a fork larger than its per-refresh byte allowance re-read the same prefix on every refresh and the scan never completed. This is pre-existing on the base; the branch's bounded test worked around it by shrinking the child fixture under a 512-byte per-file limit. Upstream continues an unfinished resolved fork at its cached offset while the parent dependency is unchanged (canResumeCodexForkAccounting, initialForkAccountingState). With upstream's fixture shape the bounded chain test never completed.
  • A fork parsed again from byte zero (for example, a finished fork that grew) started from the cached, already used-up inherited remainder instead of the full inherited baseline, so it billed inherited usage as its own. Example: root 1000, child last-only rows 600 and 20, then 500 appended. A cold scan bills 120; the warm scan billed 740. This is pre-existing since [0.63.0] Preserve paginated Codex accounting #589 (5d878f7). Upstream passes saved fork state only to a resumed parse and starts every other parse fresh.
  • The direct-fork tests did not follow upstream CodexDirectForkBaselineTests. They used compact token rows, a 512-byte per-file limit, candidate limit 0, oldest-first order and forced mtimes. Upstream uses full event_msg token_count rows and bounds only the per-refresh byte budget.

What changed (merge of the base at 15f1091, then two commits)

  • CodexForkAccountingState.resume (serde default, omitted when absent) stores the parser state that the per-file cache fields do not already carry: the parent baseline the parse started from, the totals watermark, the interleaved flag and the paginated-baseline flag. It is cleared once the parse reaches its target.
  • New cost_scanner/codex/fork_resume.rs (75 lines) decides when a cached fork may continue at parsed_bytes. It requires matching session, fork parent, history base and fork time (the existing check), an unchanged file identity, the same validated parent baseline, a state that is not locally resolved, a cached entry that is not an unresolved parent, a cursor past zero on a line boundary, and a resumable scan target. Any other fork is parsed from byte zero.
  • jsonl_scanner/codex/parser.rs: a ResumeParentBaseline mode restores the model, totals, timestamps, first token timestamp, the effective inherited baseline (after any paginated raise), the remaining inherited counters verbatim (None means used up) and the flags above.
  • cost_scanner/codex.rs: a resumed parse merges its suffix into the cached day map, bills the merged map (as the non-fork resume path does) and counts as files_resumed. A parse from byte zero passes no remaining counters, and remaining_inherited_totals is removed from CodexAccountingMode::Baseline (logical_target.rs).
  • Tests:
    • direct_fork.rs uses upstream's fixture shape and asserts that a bounded scan reads every history byte exactly once.
    • New fork_resume.rs: used-up counters stay used up across resumed passes, a changed parent baseline restarts the parse, and a grown fork's reparse starts from the full baseline.
    • paginated.rs: a bounded paginated continuation raises its baseline once.
  • CHANGELOG: one line for the resume and the grown-fork fix.

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)

  • Changed root: the child fails closed (unresolved) instead of rebilling 10 as upstream does. This is unchanged from the branch.
  • Subagent forks in parent-baseline mode resume here. Upstream resumes subagent threads only through its buffered-line path. This port restores the full parser state, and a lineage or identity change still forces a parse from byte zero.

Commands and results (toolchain 1.98.0, run in the lane-a worktree through the build gate)

  • cargo +1.98.0 fmt --all --check: clean
  • cargo +1.98.0 clippy --workspace --all-targets -- -D warnings: pass
  • cargo +1.98.0 test -p codexbar --lib cost_scanner: 127 passed, 0 failed
  • cargo +1.98.0 test -p codexbar --lib jsonl_scanner: 69 passed, 0 failed
  • cargo +1.98.0 test -p codexbar --lib -- direct_fork fork_resume paginated copied_prefix lineage_cache: 40 passed, 0 failed, six runs in a row
  • cargo +1.98.0 test -p codexbar: 2245 passed, 0 failed, 1 ignored
  • cargo +1.98.0 test -p codexbar-desktop-tauri: 477 passed, 1 failed: bootstrap_payload_exposes_every_provider_variant. This is the known catalog-size drift (Isolate bootstrap payload test from real settings #684, fixed by Make the bootstrap catalog test hermetic (#684) #711); the same failure is on the base, and this PR does not touch the provider catalog.
  • 05ebe7f only adds the CHANGELOG line on top of the validated 2f29161.

No frontend, locale or bridge change, and no new dependencies. Changed source files stay under 1000 lines (parser.rs 957, jsonl_scanner.rs 962); tests.rs was already over and gains 3 lines. Pushed as fast-forwards (8e78ad1..05ebe7f). Backend only, so no UI proof is needed.

@Finesssee

Copy link
Copy Markdown
Collaborator Author

Shipped in v0.70.0: this PR's head is included in main via #735 (merge commit 9d0a37a). Closing as integrated.

@Finesssee Finesssee closed this Oct 3, 2026
junglesub-bot Bot pushed a commit to junglesub/Win-CodexBar that referenced this pull request Oct 4, 2026
…origin for direct Codex fork chains (stacked on nesszer#610)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant