Inject EGG_REAL_BACKEND_WINDOW into agent event pods (#3316) - #3330
Conversation
The #3249 context-discipline measurement's window_utilization and real_backend_window metrics were permanently null in production: the in-pod measurement (shared/egg_agent/measurement.py) resolves the real backend window from the $EGG_REAL_BACKEND_WINDOW cross-boundary env, but no producer wired it. The orchestrator is off the pod's PYTHONPATH, so the fallback import can't compute it either, leaving both window-relative metrics null. Wire the symmetric counterpart to the #3279 EGG_RESEED_THRESHOLD injection: _build_event_spawn_params now also resolves real_backend_window(decision.claude_code_alias) from the same model decision it already computes, and spawn_event exports it as EGG_REAL_BACKEND_WINDOW alongside EGG_RESEED_THRESHOLD. The real window is returned separately (not recovered from the reseed threshold) because the threshold is min(FLOOR, MARGIN * real_window) and so is not invertible once the floor binds. Tests mirror the three reseed cases: default Claude (1M), sub-1M LiteLLM (kimi-k2.7-code -> 262144, not the [1m]-implied 1M), and unregistered LiteLLM (conservative 200000). Docstrings in measurement.py updated to reflect that the producer is now wired.
ae7d03f to
ce2cec7
Compare
…-real-backend-window # Conflicts: # orchestrator/concurrent_executor.py # orchestrator/tests/test_concurrent_executor.py
This comment has been minimized.
This comment has been minimized.
Add the #2548 cross-reference to docs/reference/orchestrator-cli.md and tie the egg/<id>/context branch literal to a slice-1 mention in the same paragraph in docs/guides/concurrent-execution.md, satisfying the documentation-pinning tests in tests/docs/test_context_pr_doc_terminology.py.
Autofix tracking{"Test/Unit Tests": 1} |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review: Inject EGG_REAL_BACKEND_WINDOW into agent event pods (#3316)
Verdict: Approve. This is a clean, correctly-scoped fix for a genuine cross-module silent no-op. I traced the full producer→consumer data flow and verified every resolved value against the registry. No blocking issues. One non-blocking note on scope.
What I verified
The bug is real and the fix addresses it end-to-end. This is the canonical cross-module dead-end the review rules call out, fixed: measurement.py:_resolve_real_window reads $EGG_REAL_BACKEND_WINDOW first (line 229) and only then falls back to importing orchestrator.agent_model_resolution, which always fails in-pod because orchestrator is off the sandbox PYTHONPATH. With no producer exporting the env, both real_backend_window and window_utilization were permanently null (consistent with the live evidence cited from issue-3288). The PR wires the producer side in spawn_event, closing the gap. Consumer constant REAL_BACKEND_WINDOW_ENV = "EGG_REAL_BACKEND_WINDOW" (measurement.py:101) matches the exported key exactly.
Resolution chain is correct for all three branches (traced classify_model → claude_code_alias → real_backend_window):
- Default opus →
claude_code_alias="opus"→_is_claude_alias→1_000_000✓ kimi-k2.7-code→ in_SUB_1M_CONTEXT_MODELS, soclaude_code_aliasstays bare → registry lookup →262_144(not the[1m]-implied 1M) ✓qwen3-coder-30b→ unregistered,claude_code_alias="qwen3-coder-30b[1m]"→real_backend_windowstrips[1m], not in registry, not Claude →_conservative_unknown_window()→200_000✓
The "returned separately, not recovered from reseed_threshold" rationale holds. reseed_threshold = min(400_000, int(0.80 * real_window)); once the 400k floor binds (any real_window >= 500_000, including the default 1M) the real window is genuinely not recoverable from the threshold. Returning it as a distinct 5th tuple element is the right call, not redundancy.
Tuple arity change (4→5) is fully propagated. grep confirms the only production caller (spawn_event, line 180) and the one direct test (line 1684) are both updated. No silently-broken unpackers.
Tests exercise the production path and are not self-seeded. TestEventSpawnRealBackendWindow goes through the real spawn_event → _build_event_spawn_params → _resolve_model_decision → real_backend_window chain and asserts on extra_env, mirroring the proven EGG_RESEED_THRESHOLD tests. The expected values (1000000/262144/200000) are independently derivable from the registry, not pinned from a regenerated golden. Ran the new class locally: 3 passed.
Non-blocking
- Scope creep in the doc changes. The edits to
docs/guides/concurrent-execution.mdanddocs/reference/orchestrator-cli.mdare about the #2548 context-PR stack shape and thepr.context_pr_numberfield — unrelated to theEGG_REAL_BACKEND_WINDOWinjection. They read as accurate and harmless, but they don't belong in a #3316 PR; bundling unrelated doc touch-ups makes the change history harder to bisect later. Consider splitting them out (not a merge blocker).
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Response to review feedbackThanks for the thorough trace — verdict was Approve with one non-blocking note. One actionable item, dispositioned below. 1. "Scope creep in the doc changes" — split out the
|
This comment has been minimized.
This comment has been minimized.
…-real-backend-window # Conflicts: # docs/guides/concurrent-execution.md # docs/reference/orchestrator-cli.md
There was a problem hiding this comment.
Re-review: EGG_REAL_BACKEND_WINDOW injection (#3316)
No new issues. This PR is already merged, and the delta since my prior review (54227dbe) contains no new PR-authored code — git log 54227dbe..HEAD --not origin/main is empty. The only new commit is the merge 33934fb integrating origin/main.
What I checked in this delta:
- Feature code untouched.
orchestrator/concurrent_executor.py,shared/egg_agent/measurement.py, andtest_concurrent_executor.pyare unchanged since my approval. The producer→consumer wiring (spawn_eventexportingEGG_REAL_BACKEND_WINDOW↔measurement.pyreadingREAL_BACKEND_WINDOW_ENV) and the three resolution branches (1M / 262144 / 200000) all stand. - Conflict resolution is sound. The merge resolved conflicts only in
docs/guides/concurrent-execution.mdanddocs/reference/orchestrator-cli.md. Both are doc-only and correctly integrate both sides (cross-referencing#2548, adjusting an anchor link). No semantic regression. - Merge didn't break the pinned doc tests. The conflict resolution re-worded the exact paragraphs pinned by
tests/docs/test_context_pr_doc_terminology.py. I ran that suite against the merged docs:30 passed, 9 xpassed— theslice-1↔egg/<id>/contextco-occurrence pin and the#2548cross-reference pin both still hold. - Prior non-blocking note dispositioned. My earlier "doc scope creep" suggestion was reasonably answered: those edits keep CI green against a pre-existing
maindoc-drift, so splitting them out would have required a merge-ordering dependency.
Verdict unchanged from my prior review: clean, correctly-scoped fix. Nothing blocking.
— Authored by egg
|
egg review completed. View run logs 5 previous review(s) hidden. |
Summary
Fixes #3316. The #3249 context-discipline measurement's
window_utilization(peak-utilization-under-resume) andreal_backend_window(working-set-vs-window, the #3200 §6 recursion-escalation signal) were permanentlynullon every production event because no producer wired theEGG_REAL_BACKEND_WINDOWcross-boundary env into agent event pods.shared/egg_agent/measurement.pyresolves the real backend window from$EGG_REAL_BACKEND_WINDOWfirst, then falls back to importingorchestrator.agent_model_resolution.real_backend_window. Butorchestratoris off the pod'sPYTHONPATH, so the fallback always fails in-pod — leaving both window-relative metrics null (confirmed live onissue-3288: all 24 captured eventsnullfor both).Fix
The symmetric counterpart to the #3279
EGG_RESEED_THRESHOLDinjection:_build_event_spawn_paramsnow also resolvesreal_backend_window(decision.claude_code_alias)from the same model decision it already computes, returning it in the tuple (4 → 5 elements).spawn_eventexportsenv["EGG_REAL_BACKEND_WINDOW"] = str(real_window)alongside the existingEGG_RESEED_THRESHOLDexport.The real window is returned separately rather than recovered from the reseed threshold, because the threshold is
min(FLOOR, MARGIN * real_window)and so is not invertible once the floor binds.Tests
New
TestEventSpawnRealBackendWindowmirrors the three reseed cases:1000000kimi-k2.7-code) →262144(not the[1m]-implied 1M)qwen3-coder-30b) → conservative200000Plus the existing
_build_event_spawn_paramstest now asserts the 5th tuple element. Stale "no producer wires it yet" docstrings inmeasurement.pyupdated.Notes
EGG_RESEED_THRESHOLDinjection).🤖 Generated with Claude Code