[issue-3200][slice-1/10] Token-occupancy capture (prerequisite, AC-1)

[james-in-a-box]

Capture cumulative window occupancy (cache_read+cache_creation+input) in AgentResult from ResultMessage.usage. Depends on: none. Blocks phases 8 and 10.

Base PR: #3234

What's in this PR

Commits (3):

• Persist BRC history for slice-1 (Slice PRs are missing analysis/plan docs and all BRC history; need a 'context' PR for refine+plan phases plus per-slice BRC in each slice PR #2548)
• implement(BRC context discipline: protected directive root + queryable environment, bounded by a deterministic threshold reseed #3200): test window-occupancy capture on AgentResult (slice-1, task-1-3)
• implement(BRC context discipline: protected directive root + queryable environment, bounded by a deterministic threshold reseed #3200): capture window occupancy on AgentResult (slice-1, AC-1)

.egg-state/brc-history/3200-implement-slice-1.json | 720 +++++++++++++++++++++++++++++++++++
 .egg-state/brc-history/3200-implement-slice-1.md   | 826 +++++++++++++++++++++++++++++++++++++++++
 shared/egg_agent/client.py                         |  63 ++++
 shared/egg_agent/result.py                         |  18 +
 tests/shared/egg_agent/test_client.py              | 251 ++++++++++++-
 5 files changed, 1877 insertions(+), 1 deletion(-)

This slice

Token-occupancy capture (prerequisite, AC-1)

Files affected:

• shared/egg_agent/result.py
• shared/egg_agent/client.py
• shared/egg_agent/tests/

Tasks (3) + acceptance criteria

• task-1-1: Extend AgentResult (shared/egg_agent/result.py:24-33) with an optional cumulative window-occupancy field (default None, non-breaking). Docstring defines occupancy = cache_read + cache_creation + input and states it is NOT billed/effective input. Optionally retain raw component counts for downstream breakout; the single total is the load-bearing field.
  • Acceptance criteria: Optional occupancy field defaults None; docstring defines occupancy and excludes billed input; existing constructors still build.
• task-1-2: In shared/egg_agent/client.py stop dropping message.usage on the ResultMessage branch (717-751). Read usage, compute occupancy defensively (missing/None sub-fields -> 0), thread it into result_meta and EVERY AgentResult build site in the file (~733/~747/~760/~782/~804/~825). Guard against SDK shapes with no usage.
  • Acceptance criteria: usage read on the ResultMessage branch; occupancy=cache_read+cache_creation+input computed defensively; populated on all AgentResult return sites; absent usage -> None, no exception.
• task-1-3: Unit tests for occupancy capture: populated usage -> sum; absent usage -> None without raising; partial usage -> sum of present components; a cache-dominated case proves cache_read is included (not just input).
  • Acceptance criteria: full/absent/partial cases covered; cache-dominated case asserts occupancy != billed input; tests pass under make test.

Stack

• Position: slice 1 of 10 in pipeline issue-3200
• Base PR: BRC context discipline (#3200) — pipeline state branch (implementation in slice PRs #3236–#3243) #3234
• Stacked on top of egg/issue-3200/work

[james-in-a-box]

Contract re-verification — PR #3236 (slice-1/10, AC-1: token-occupancy capture)

Verdict: Approve (re-affirm). Incremental verification against my prior review at 102a4a12.

Delta since last review

The only PR-authored change since 102a4a12 is the merge commit 8835a00, which merges origin/egg/issue-3200/work (base) into the PR branch and resolves a conflict in .egg-state/contracts/issue-3200.json by taking the base branch's authoritative pipeline state.

• Slice-1 implementation is unchanged. git diff 102a4a12 HEAD -- shared/egg_agent/ tests/shared/egg_agent/ is empty. result.py, client.py, and test_client.py are byte-identical to what I verified last cycle, so the previously-verified AC-1 implementation still holds (no regression).
• The orchestrator files in the raw delta are merge-ins, not new PR work. decisions.py, pipelines.py, and test_orphaned_driver_revival.py at HEAD are identical to the base-branch tip (f592b01); they entered via the merge. The effective PR diff (origin/egg/issue-3200/work...HEAD) is cleanly scoped to slice-1: shared/egg_agent/client.py, shared/egg_agent/result.py, tests/shared/egg_agent/test_client.py (+ BRC history). They do not widen the PR's effective scope.
• Contract conflict resolution is sound. The resolved contract keeps slice-1 = complete with correct task→commit linkage: task-1-1 and task-1-2 → d0705780. No previously-verified state was rolled back.

Re-confirmed evidence (intact in current tree)

• shared/egg_agent/result.py:53-54 — window_occupancy: int | None = None and token_usage: dict | None = None, both optional/non-breaking, docstring defines occupancy = input + cache_read + cache_creation and excludes billed input (task-1-1).
• shared/egg_agent/client.py:76 _compute_occupancy() + last_assistant_usage sourced from the final top-level turn (parent_tool_use_id is None, line 732) and threaded into every AgentResult build site (success ~833, and error/timeout/SDK/generic sites ~857/881/905). Absent/partial usage → None/0-filled, never raises (task-1-2).
• tests/shared/egg_agent/test_client.py:1679,1712 — TestAgentResultOccupancyField + TestOccupancyCapture cover defaults, full/absent/partial usage, cache-dominated (occupancy ≠ billed input), final-turn-not-aggregate, sub-agent exclusion, and error-site capture (task-1-3 coverage present in-tree).

Notes for the human reviewer (non-blocking, pre-existing)

• task-1-3 is still marked pending in the contract even though its test coverage exists in-tree. This is a contract-bookkeeping discrepancy that predates this delta (owned by the tester role), not introduced by the merge — flagging for completeness, not as a delta violation.
• The orchestrator is unreachable, so verify-criterion could not be run this cycle. The contract's top-level acceptance_criteria list is empty; slice-1's criteria are tracked at the slice/task level and are already marked complete.

No contract violations introduced by the delta. Previously-verified criteria still hold.

— Authored by egg

[egg-reviewer]

Re-review: [issue-3200][slice-1/10] Token-occupancy capture — sub-agent filtering added ✅

I re-reviewed the current PR head (the prior-review commit 102a4a12 was squashed/rebased out of the branch, so I traced the full current diff of the three code files against the base rather than a git-range delta). The delta since my last approval is a single, focused addition: sub-agent (Task tool) turns are now excluded from last_assistant_usage — exactly the non-blocking concern I flagged in my prior approval. It is implemented correctly, with a real test. Approving.

🟡 (prior non-blocking) sub-agent messages polluting last_assistant_usage — RESOLVED

client.py:730-734 now gates the per-turn capture on both a present usage block and a top-level turn:

if (
    message.usage is not None
    and getattr(message, "parent_tool_use_id", None) is None
):
    last_assistant_usage = message.usage

I verified this is correct at the SDK level, not just structurally:

• claude_agent_sdk/types.py:1030 declares AssistantMessage.parent_tool_use_id: str | None, and _internal/message_parser.py:175,177 populates it from data.get("parent_tool_use_id") alongside usage=data["message"].get("usage") (the raw per-API-call window block). Sub-agent turns carry a non-None parent_tool_use_id; top-level turns are None (types.py:1544 — "Always None for top-level conversation"). So filtering to parent_tool_use_id is None keeps only the main session's resident window — the value the reseed decision needs.
• getattr(..., None) is appropriately defensive against SDK shapes lacking the attribute (and the test's mock AssistantMessage gained parent_tool_use_id so the CI mock-SDK path exercises the same branch).

I hand-traced the capture/occupancy logic against the three relevant cases (main-then-subagent terminal turn, multi-step, all-subagent) and all resolve as intended: a trailing sub-agent turn no longer overwrites the main window; an all-sub-agent stream falls back to None → safe reseed.

🟡 (prior non-blocking) test couldn't surface the sub-agent overwrite — RESOLVED

test_subagent_turns_excluded_from_occupancy drives the real production path (run_agent_async → stream loop → _compute_occupancy): it streams a main-session turn (150k window) followed by a sub-agent turn (parent_tool_use_id="task-abc", 30k window) before the ResultMessage, and asserts window_occupancy == 150_000 with an explicit != 30_000 guard. Because the sub-agent turn is the terminal AssistantMessage, this test fails against the pre-fix code (which would capture 30k) — it pins the behaviour, not the factoring. No hand-built fixture bypasses the helper; no self-seeding golden.

What's still correct (carried over from my f3f42b3 approval)

• The original blocking fix is intact: occupancy is sourced from the final AssistantMessage.usage (per-turn window), never the cumulative ResultMessage.usage. test_multistep_uses_final_turn_not_cumulative_aggregate still guards the aggregate regression (asserts 150k, != 373k).
• All five build sites thread _compute_occupancy(last_assistant_usage) / _usage_components(last_assistant_usage) (success :931, is_error :806/833, timeout :857, Process/SDK error :881, generic Exception :905); the pre-flight ImportError site keeps the None default. Error/timeout sites report the last-known window rather than dropping to None.
• _compute_occupancy / _usage_components stay defensive: non-dict → None; missing/None/non-int subfields → 0; the not isinstance(value, bool) guard excludes bool.
• New fields are additive trailing optionals (None default); test_existing_positional_construction_still_builds pins non-breaking construction. Field is unconsumed in slice 1, so no runtime regression.
• except TypeError, ValueError: near the changed code (client.py:740) is pre-existing (not in this diff) and valid Python 3.14 (PEP 758).
• CI is green (Unit Tests, Integration Tests, Security Scan all SUCCESS).

Non-blocking observation (no change required)

The added message.usage is not None guard means a top-level turn whose usage is None no longer resets last_assistant_usage — it sticks to the last turn that did report usage. In practice the terminal assistant turn always carries usage, and where every turn lacks it (e.g. usage-free LiteLLM routes) the field stays None → safe reseed, so this only ever biases toward retaining a real window estimate, which is the conservative direction for the reseed decision. Fine as-is; just flagging the behavioural nuance for whoever wires up the consumer in phase 8/10.

Clean slice. The source was already right; the sub-agent edge is now closed before nine downstream slices build on this value.

— Authored by egg

[james-in-a-box]

Automated feedback loop has reached the maximum of 5 rounds. Human review is needed to make further progress on this PR.

[james-in-a-box]

Automated feedback loop has reached the maximum of 5 rounds. Human review is needed to make further progress on this PR.

[james-in-a-box]

Conflict Resolution Summary

Resolved merge conflicts with egg/issue-3200/work (merge, not rebase — history preserved, revertable).

File	Category	Resolution
.egg-state/contracts/issue-3200.json	State-file progression (additive)	Took the base branch's canonical accumulated pipeline state

Details: All 10 conflict regions followed one pattern. This PR's branch (HEAD) carried stale bookkeeping for the later slices — slices 8/9/10 pending, null commits, empty notes, null PR numbers, and a shorter audit log. The base branch (egg/issue-3200/work) is the integration branch and canonical state accumulator, carrying the progressed state (those slices complete with their commit SHAs, implementation notes, PR numbers 3251/3252, and the additional audit-log entries). Took the base version for every conflict region. This is a state-file progression, not a semantic source conflict — no production logic was in conflict.

Verified the resolved contract preserves this PR's own state: slice-1 -> complete | pr 3236, all 10 slices intact.

Verification:

• Contract JSON re-validated as well-formed.
• ruff check + ruff format clean across the merged tree (906 files).
• No conflicts in any source file — only the JSON state file conflicted.

Please review — pre-existing, NOT introduced by this merge: make lint's mypy step reports 3 errors in shared/egg_agent/client.py (lines 399, 486-487: options.effort Literal assignment and check_builtin_tool_output_risk arg types). These exist verbatim in the pre-merge PR tip — the merge did not touch client.py or those definitions (the base branch doesn't contain window_occupancy yet; this slice is the prerequisite that adds it). They are out of scope for conflict resolution and left untouched. CI will run the full suite on the pushed result.

— Authored by egg

[egg-reviewer]

Re-review: [issue-3200][slice-1/10] Token-occupancy capture — merge-only delta, source unchanged ✅

I re-reviewed the current PR head (97ba3df) against my prior approval at 8835a00. The delta since my last review contains no source-code changes.

What changed since 8835a00

The only PR-authored commit in 8835a00..HEAD --not origin/egg/issue-3200/work is the merge commit 97ba3df, whose stated purpose is resolving a conflict in .egg-state/contracts/issue-3200.json. I verified the scope precisely:

• The three code files are byte-identical to my last approval. git diff 8835a00..97ba3df -- shared/egg_agent/client.py shared/egg_agent/result.py tests/shared/egg_agent/test_client.py is empty. The occupancy capture logic, the five build sites, the sub-agent filtering, and the test suite I approved at 8835a00 are unchanged.
• All orchestrator/, docs/, gateway/, integration_tests/, sandbox/ changes in the range came from the merged base tip, not this PR. git diff 3ae1f6b..97ba3df over those trees is empty — HEAD matches the base branch exactly for them. They are out of scope for this slice-1 review (reviewed via their own PRs).
• The only PR-unique files vs base are two .egg-state/brc-history/ artifacts — pipeline state managed by the orchestrator, excluded from review per the review rules.

Conflict resolution is correct

The resolution took the base branch's canonical accumulated pipeline state for all conflict regions while preserving slice-1's own bookkeeping. Confirmed in the merged contract: slice-1 → complete | pr 3236, and all 10 slices intact with their commit/PR state. This is an additive state-file progression, not a semantic source conflict — no production logic was in conflict. Per the review rules (.egg-state/ artifacts are orchestrator-managed and skipped), there is nothing blocking here.

Carried-over correctness (unchanged from 8835a00)

The substance I approved still holds: occupancy is sourced from the final per-turn AssistantMessage.usage (never the cumulative ResultMessage.usage); sub-agent turns are filtered via parent_tool_use_id is None; all error/timeout/success build sites thread the value; _compute_occupancy/_usage_components stay defensive (non-dict → None, missing/None/non-int → 0, bool excluded); the new AgentResult fields are additive trailing optionals defaulting to None. Tests pin behaviour against the production path, not hand-built fixtures or self-seeding goldens.

CI

Green on HEAD — Unit Tests, Integration Tests, Security Scan, and all lint suites (Python, Shell, YAML, Docker) SUCCESS; aggregate SUCCESS.

The pre-existing mypy notes flagged in the conflict-resolution summary (client.py:399, 486-487) sit outside the changed regions, are type-annotation issues the review rules explicitly skip, and are not failing CI.

Nothing new to scrutinize and nothing has regressed. Approving.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

22 previous review(s) hidden.

[james-in-a-box]

Contract Re-Verification — PR #3236 (slice-1, AC-1)

Verdict: Approve — no contract violations, no regressions since my last review at 8835a00a.

Scope of this incremental re-review

The only PR-authored commit since 8835a00a is the merge 97ba3df ("Merge origin/egg/issue-3200/work into egg/issue-3200/slice-1"). Verified via git log 8835a00a..HEAD --not origin/egg/issue-3200/work — a single merge commit. Everything else surfaced by a plain two-dot diff is base-branch content pulled in by the merge, not slice-1 work.

What the merge actually changed (the conflict resolution): .egg-state/contracts/issue-3200.json only. It adopted the base branch's canonical accumulated pipeline state — slices 8/9/10 flipped pending → complete with their commits/notes/PR numbers (#3251, #3252) and the corresponding audit-log entries. This is bookkeeping for downstream slices; it does not touch slice-1's deliverable or its AC-1.

Regression check — slice-1 deliverable intact

The three slice-1 source files are byte-identical to my last review (git diff 8835a00a..HEAD empty for each):

• shared/egg_agent/result.py — window_occupancy / token_usage fields
• shared/egg_agent/client.py — occupancy capture
• tests/shared/egg_agent/test_client.py — occupancy tests

The true PR delta vs base (origin/egg/issue-3200/work...HEAD) remains focused: client.py (+91), result.py (+21), test_client.py (+359), plus BRC history artifacts.

AC-1 still holds (re-confirmed)

• task-1-1 (occupancy field): AgentResult.window_occupancy: int | None = None defaults None, docstring defines occupancy = cache_read + cache_creation + input and explicitly excludes billed input; constructors non-breaking. ✓
• task-1-2 (capture): _compute_occupancy / _usage_components coerce missing/non-int sub-fields to 0; absent/non-dict usage → None (no raise). Occupancy is sourced from the final top-level AssistantMessage.usage (sub-agent turns filtered via parent_tool_use_id is None), not the cumulative ResultMessage.usage — a documented correctness improvement over the literal task text (#3200), and threaded into all return sites including the timeout and error paths. ✓
• task-1-3 (tests): coverage present for every required case — test_full_usage_sums_window_components, test_absent_usage_yields_none_without_raising, test_partial_usage_sums_present_components, test_cache_dominated_turn_includes_cache_read (proves cache_read is counted, not just input), plus test_output_tokens_excluded_from_occupancy, test_subagent_turns_excluded_from_occupancy, and test_error_result_also_captures_occupancy. ✓

Notes for the human reviewer

• Contract bookkeeping lag (non-blocking, unchanged since last review): task-1-3 status is pending in the contract even though its tests exist and pass; slice-1 itself is status: complete. Cosmetic — the implementation and tests are present.
• Criterion marking unavailable this run: the orchestrator is UNREACHABLE (gateway healthy), so egg-contract verify-criterion could not be invoked. The contract also carries no top-level acceptance_criteria[] entries (ACs are embedded per-task), so there is no ac-N to mark; verification above is against the per-task acceptance criteria text.

— Authored by egg

[james-in-a-box]

egg contract-verification completed. View run logs

22 previous review(s) hidden.

[james-in-a-box]

Automated feedback loop has reached the maximum of 5 rounds. Human review is needed to make further progress on this PR.