Repository navigation
Conversation
…toring Also replace the 'always use make test' guidance with targeted .venv/bin/pytest for the inner loop; make test can take 10+ minutes on wide diffs and pulls in known host-environment failures (#3222).
![egg-reviewer[bot]](https://avatars.githubusercontent.com/u/8340608?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed as a docs-only change to CLAUDE.md. Since this PR's whole purpose is to bake "durable facts that sessions repeatedly get wrong" into the authoritative context file, I verified every path, filename, and behavioral claim in the new sections against the codebase. Most check out; one is wrong and one is imprecise.
Blocking
1. shared/egg_harness/config.py does not exist — the egg_harness package was removed.
No model snapshot pins in defaults. … canonical mapping in
shared/egg_harness/config.py…
There is no egg_harness package anywhere in the repo. The removal is documented in docs/architecture/integration-test-trust-boundary.md:196:
ScriptedProviderwas a unit-test-only LLM provider double that lived in theegg_harnesspackage; the package was removed when the custom-harness experiment ended.
The canonical model-alias mapping actually lives in orchestrator/agent_model_resolution.py — see _CLAUDE_EXACT_ALIASES ({"opus", "opus[1m]", "sonnet", "sonnet[1m]", "haiku", …}), DEFAULT_AGENT_MODEL, and the overseer tier map (classify→haiku, routine→sonnet, adversarial→opus).
This matters more than a normal typo: it sits in the "Architecture Invariants" section that explicitly instructs future sessions to "verify against code." A session that greps shared/egg_harness/config.py finds nothing and is left worse off than with no pointer at all — the section defeats its own purpose. Please point it at orchestrator/agent_model_resolution.py (the same file the auto-compact bullet already cites for compaction profiles).
Non-blocking
2. sandbox/entrypoint.py is a package, not a module file.
Kubernetes bypasses the image ENTRYPOINT. … so
sandbox/entrypoint.pynever runs on the k8s path …
The entrypoint is a package directory (sandbox/entrypoint/ with __init__.py, __main__.py, _core.py, …), not a single sandbox/entrypoint.py file. The claim itself is correct — kubernetes_spawner/_spawn.py passes an explicit command= (line 701), so the image ENTRYPOINT is bypassed and boot setup belongs in the spawner. Just refer to sandbox/entrypoint/ so the path resolves.
Verified accurate (no action needed)
For the record, the following claims were checked and hold:
orchestrator/tests/test_wait_instruction_ratchet.pyexists (agents-never-wait invariant).scripts/push-egg-images.shrefuses non-loopback registries and intentionally has no override (lines 44–49).ScriptedProviderunit-test-only claim matchesdocs/architecture/integration-test-trust-boundary.md.shared/egg_anchor/,orchestrator/agent_model_resolution.py,sandbox/scripts/confluence,docs/guides/testing.md,CONTRIBUTING.mdall exist.make test-allexists and records LKG on green (Makefile:407-408).- ~95% auto-compaction and compaction-profile management confirmed in
orchestrator/agent_model_resolution.py:186,527. - Phase-gate parsing (
orchestrator/routes/pipelines.py:_parse_resolution,_APPROVE_KEYWORDS = {"approved","approve","lgtm","yes",""}) matches the invariant, including "JSON approve-with-feedback silently drops the feedback" —action in ("approve","select","submit_feedback")returns(True, None), discardingfeedback.
Fix item 1 (a one-path change) and this is good to merge.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Point model-alias mapping at orchestrator/agent_model_resolution.py (egg_harness package was removed) and refer to the entrypoint as the package dir sandbox/entrypoint/ rather than a nonexistent .py file.
|
Thanks for the thorough path verification. Both items fixed in commit 1. 2. Ran — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review of commit 2361a57 (delta since 7cd260c).
Both prior findings resolved and verified
The single new commit is a clean, markdown-only fix that addresses exactly the two path issues from my prior review. I re-verified each against the codebase rather than taking the fix at face value:
1. Model-alias mapping path (was blocking). CLAUDE.md:52 now points at orchestrator/agent_model_resolution.py — _CLAUDE_EXACT_ALIASES, DEFAULT_AGENT_MODEL.
- Confirmed
shared/egg_harness/no longer exists (the old path was dead). - Confirmed the cited symbols exist there:
DEFAULT_AGENT_MODEL = "opus"(line 57) and_CLAUDE_EXACT_ALIASES(line 154). - Confirmed the aliases the bullet names —
opus,sonnet,haiku— are all members of_CLAUDE_EXACT_ALIASES, so the surrounding claim still reads true.
2. Entrypoint path (was non-blocking). CLAUDE.md:66 now refers to sandbox/entrypoint/ (package dir) instead of the nonexistent sandbox/entrypoint.py.
- Confirmed
sandbox/entrypoint.pydoes not exist andsandbox/entrypoint/is a package (__init__.py,__main__.py,_core.py, …). Path now resolves.
No new content was introduced and no other claims changed, so the rest of the previously-verified invariants remain accurate. Nothing else to flag.
LGTM.
— Authored by egg
|
egg review completed. View run logs 3 previous review(s) hidden. |
Consolidates recurring session-learned guidance into CLAUDE.md so fresh sessions stop re-deriving (or getting wrong) the same facts.
Changes
Testing guidance updated. The inner-loop default is now targeted
.venv/bin/pyteston the suites a diff touches, plusmake lint.make testremains available but can take 10+ minutes on wide diffs and pulls in the known host-environment failures (#3222); full-suite runs stay in CI.New Conventions section:
egg/branch prefix, public-repo hygiene for issues/PRs/comments, no model snapshot pins in config defaults, PR sizing and scoping, follow-up issue framing, and problem-not-decomposition framing for pipeline feeder issues.New Architecture Invariants section, durable facts sessions repeatedly get wrong: agents never wait on the bus (wrapper owns waiting), sandbox never writes host mounts, images never leave the host, ScriptedProvider is unit-test only, k8s bypasses the image ENTRYPOINT, agents do auto-compact (~95% of window), sandbox has no Confluence/Jira MCP (CLI wrappers only), and the one-context-PR slice topology.
New Pipeline Monitoring and HITL section: how to read early BRC silence and heartbeat silence during drafting, single-cycle consensus expectation, NACK cycle count is not a model-quality signal, strict phase-gate resolution parsing (only bare approve advances), never auto-resolving HITL decisions, and pairing failing tests with NACKs that name them.