docs: rewrite main README around four principles; retire stale architecture claims - #3588
Conversation
…hitecture The README's structure dated from the #2900 rewrite and no longer matched the system: the overseer section described the retired respawning watcher pod instead of the #2270 detection-plane + on-demand adjudicator model, the architecture diagram implied resident agent cohorts instead of the #3064 orchestrator-owned event loop with one-shot per-event pods, and the BRC section predated the #3523 review-quality layer and #2908 durable memory. Reorganize around four principles (structural human authority; evidence- based review; mechanics in code, judgment in models; context as a budgeted resource), add previously uncovered capabilities (context discipline, review-quality machinery, multi-repo pipelines, GitHub automation suite, work-preservation/resilience, Redis), and fix stale facts (repo layout, branch prefixes, slice-concurrency defaults).
…fectiveness The doc still described the removed @mention trigger, GitHub Actions workflow logs as the audit trail, and the interactive CLI mode (removed in #1762). Update the flow to the MCP submit_task pipeline, point the audit trail at issue artifacts, PR deliberation, and persisted agent transcripts, and replace the interactive-CLI paragraph with the headless-agents posture.
![egg-reviewer[bot]](https://avatars.githubusercontent.com/u/8340608?s=60&v=4){kind=small}
There was a problem hiding this comment.
No agent-mode design concerns.
This is a documentation-only change (README.md + docs/architecture/collaboration-effectiveness.md). It touches no prompt-assembly code, model identifiers, structured-output schemas, pre-fetching logic, or post-processing pipelines — nothing on the agent-mode design surface. The tier references in the prose ("Opus tier", "Haiku classifier") are descriptive aliases, not pinned model IDs, and several are being reworded/removed here rather than introduced.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review: README rewrite + collaboration-effectiveness fix
I reviewed this as a documentation-accuracy audit: every concrete, checkable claim in the new text against the code and the linked docs, plus link resolution. This is a high-quality, well-researched PR — I could not find a blocking issue. Approving with one non-blocking note.
What I verified against the tree
Links (the PR body claims "every relative link was checked"): Confirmed. All 39 unique relative link targets in the new README resolve to existing files, and the docs/guides/github-automation.md#documentation-onboarding anchor matches the ## Documentation Onboarding heading.
MCP tool names listed in the "Driving Pipelines" section — all real (orchestrator/mcp_tools/_dispatch.py, _tool_defs.py): update_pipeline_config, list_agent_local_commits, salvage_agent_commits, prune_stale_worktrees, validate_network_isolation, rebuild_and_rollout, get_agent_transcript.
Overseer rewrite — the single most substantive content change. Verified the new description against docs/architecture/overseer.md and the code:
- Corrective vocabulary
{nudge_agent, respawn_cohort, open_operator_hitl}— matchesorchestrator/routes/pipelines/_overseer.py(CORRECTIVE_ACTIONS) exactly. - "on-demand overseer agent (Opus tier) … verdict is advisory" — matches overseer.md ("On-demand adjudicator … ADVISES only (Opus tier)").
- The retired "Haiku-classifier-every-poll / Sonnet decision-maker / Opus advisor" model was genuinely pre-#2270; the overhaul (#2270) is real and documented. Correctly retired.
Architecture diagram / components table: Ports confirmed against k8s/base/ (:9850 MCP, :9849 REST, :9848 gateway + :3129 proxy, :4000 LiteLLM). Redis-as-message-store confirmed (orchestrator/message_store.py — "Redis Streams is the only backend"; k8s/base/redis-{deployment,service}.yaml).
New capability claims:
- Package-manager egress with proxy-layer token injection — confirmed (
gateway/README.md"GitHub Packages npm Read-Through (Token-Gated)";gateway/tests/test_npm_packages_readthrough.py). - Cross-repo merge gate / downstream-PR-stays-draft (#3393) — confirmed in
docs/architecture/slice-dag.md. EGG_CONTEXT_DISCIPLINEenv var, opt-in / default OFF (#3200) — confirmed (orchestrator/routes/event_prompt/_cli.py, default OFF).egg/recovered/*salvage refs — confirmed (orchestrator/agent_salvage.py,RECOVERY_BRANCH_PREFIX = "egg/recovered").- context-discipline.md terms ("protected root", "queryable environment", "threshold reseed") — all present in the linked doc.
collaboration-effectiveness.md: The @mention→CLI-era text is genuinely stale (interactive CLI removed in #1762, per CLAUDE.md and the doc set); the rewrite to submit_task / stacked-PR / persisted-transcript framing is accurate. get_agent_transcript is a real tool.
Non-blocking
Residual overseer drift in a doc this README now links to. The new "Deterministic health detection" bullet links to docs/guides/pipeline-health-monitoring.md, but that guide still opens (line 3) with the pre-#2270 framing: "Pipeline health monitoring uses a two-tier architecture … the overseer agent tier handles ambiguous situations." A reader who follows the README's new detection-plane/on-demand-adjudicator description into that guide lands on the older model. Your Notes section already flags docs/architecture/orchestrator.md and orchestrator/README.md as carrying the stale pre-#2270 description for follow-up — pipeline-health-monitoring.md's opening belongs in that same follow-up list, and arguably more so since the new README links to it directly as the authority for that bullet. Not blocking this markdown PR; noting it so the follow-up sweep catches it.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thanks for the thorough documentation-accuracy audit. Both reviews approved; one non-blocking item, addressed in-PR. Disposition1. Residual overseer drift in The guide's opening (line 3) still framed health monitoring as a pre-#2270 "two-tier architecture … the overseer agent tier handles ambiguous situations," and the new README links to this guide directly as the authority for the "Deterministic health detection" bullet — so a reader following that link landed on the retired model, exactly as flagged. Rather than push this into the deferred sweep, I corrected the entry point in this PR:
I scoped this to the entry point plus a clear staleness banner rather than a full body rewrite: the deep Tier-2 internal-architecture sections are a genuine follow-up (same sweep already noted for 2. 3. Documentation-accuracy audit (
— Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
No agent-mode design concerns.
The delta since my last review is a single documentation change to docs/guides/pipeline-health-monitoring.md that addresses my prior non-blocking note: a status banner now marks the Tier-1 deterministic detection as current/authoritative and the pre-#2270 Tier-2 "Overseer Agent" internals as being retired, and the opening paragraph is reworded to the delivered detection-plane + on-demand advisory adjudicator framing, pointing at overseer.md as authoritative. Verified the ../architecture/overseer.md target and the #tier-1-orchestrator-tripwires anchor both resolve.
Nothing on the agent-mode design surface: no prompt-assembly code, no pre-fetched diffs/logs, no structured-output schemas, no post-processing pipelines, and no pinned model identifiers — the "Opus tier"/"Haiku classifier" strings are descriptive prose, and the Haiku/Sonnet references are being retired rather than introduced.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review: delta since 71c28b3 — approved
The old review commit (71c28b3) was force-pushed away, so I reviewed the single delta commit 5bcd01f ("docs: retire pre-#2270 overseer framing in pipeline-health-monitoring opening") against the current tree. It touches one file, docs/guides/pipeline-health-monitoring.md, and does exactly one thing: it addresses my prior non-blocking note about residual pre-#2270 overseer framing at the entry point of the guide the new README links to as the authority for the "Deterministic health detection" bullet.
Verified
- Both new links resolve.
../architecture/overseer.mdexists; the intra-doc anchor#tier-1-orchestrator-tripwiresmatches the## Tier 1: Orchestrator Tripwiresheading (line 112) — GitHub slugifies that heading to exactly that anchor. - Banner content is accurate and consistent with the authoritative source. The new status banner's language — deterministic detection plane + on-demand, advisory adjudicator (Opus tier), bounded corrective vocabulary, retired-since-
#2270,overseer.mdauthoritative when the two disagree — is a faithful mirror of the status banner indocs/architecture/overseer.md. No new contradiction is introduced. - Scope is honest. The banner explicitly marks the deep Tier-2 internal-architecture sections still present further down (e.g. the
overseer_advisor_model/ Haiku-flags-anomaly text) as pre-#2270 and being retired, pointing readers tooverseer.md. This is the correct scoped fix — the entry point the README links to is now accurate, and the deferred body rewrite remains the same follow-up sweep already noted fororchestrator.md/orchestrator/README.md. My prior note is properly resolved, not papered over.
Docs-only change, no code surface touched. No blocking issues, no new suggestions.
— Authored by egg
|
egg review completed. View run logs 5 previous review(s) hidden. |
1 participant
Summary
The main README's skeleton dated from the #2900 rewrite (mid-May) and has only received incremental doc-updater patches since. Its individual facts were mostly current, but its model of the system was two eras old. This PR is a structured rewrite plus a fix for one drifted architecture doc it links to.
README changes
Reframed around four principles (replacing the five pillars), so the intent (broad, long-horizon, autonomous-but-human-directed work that stays aligned with human ideas) is the organizing story:
Retired stale architecture claims:
max_parallel_slicesfallback default corrected (4, not 1; raised in Flip event-loop ownership to the orchestrator and retire the in-pod wait arm (gated on a live BRC proving run of #3229) #3164). Knob-level detail demoted to doc links.egg-/egg/.Added previously uncovered capabilities: multi-repo pipelines with the cross-repo merge gate (#3393), the GitHub automation bot suite and reusable workflows (new "Beyond the Pipeline" section), Redis in the architecture diagram and components table, live pipeline operations (
update_pipeline_config), a "More Capabilities" table (Jira epic mode depth, model routing, observability, onboarding docs, redaction), andskills/,tests/,metrics/in the repo layout.Kept (verified accurate against the code): the pipeline diagrams and all 17 roles, Quick Start, ports, make targets, MCP tool names, Development section, license.
Side finding: collaboration-effectiveness.md drift
docs/architecture/collaboration-effectiveness.mdstill described the retired @mention trigger, GitHub Actions workflow logs as the audit trail, and the interactive CLI mode (removed in #1762). Updated the flow to the MCPsubmit_taskpipeline, pointed the audit trail at issue artifacts, PR deliberation, and persisted transcripts, and replaced the interactive-CLI paragraph with the headless-agents posture.Notes
make lintfails on main with a pre-existingorchestrator/tests/test_cli.py:5 I001import-sort error, unrelated to this markdown-only diff;lint-customand pre-commit hooks pass.docs/architecture/orchestrator.md("Pipeline health monitoring" section) andorchestrator/README.mdstill carry the pre-Overseer overhaul (open season): recalibrate signals, fix model/lifecycle/authority, close coverage gaps, clean up — and rethink the subsystem from first principles #2270 overseer description thatdocs/architecture/overseer.mdsays is being retired.