docs: Remove stale local pipeline mode references [doc-updater] - #1075
Conversation
Update docs to reflect elimination of the local pipeline session mode. Key changes: - orchestrator.md: Fix network mode auto-fallback (no longer local→local) - sdlc-pipeline.md: Remove stale mode=issue from API example; update terminology - STRUCTURE.md: Fix test file comment (no longer local/issue mode behavior) - local-quickstart.md: Replace "local pipeline" with "prompt-driven pipeline"
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review: Incomplete — Many stale references remain
The 4 files this PR touches are changed correctly. The terminology updates and the API example fix are accurate per the current codebase. However, this PR only addresses a small fraction of the stale "local mode" / "local pipeline" / "issue mode" references in the documentation.
Blocking: Incomplete cleanup
A grep across docs/ reveals 15+ remaining references to the eliminated local pipeline mode in files this PR does not touch. Shipping this PR as-is creates an inconsistent state where some docs say "prompt-driven pipeline" and others say "local pipeline" or "local mode." That's worse than the pre-PR state, where at least the terminology was internally consistent (even if stale).
docs/guides/sdlc-pipeline.md — The PR fixes 3 lines but misses 5 others in the same file:
- Line 815:
"via terminal in local mode, or checkbox in issue mode" - Line 1200:
"# Issue mode: start/attach to pipeline for a GitHub issue" - Line 1205:
"# Local mode: prompt-driven pipeline (no GitHub)" - Line 1209:
"Issue mode requires the -r/--repo flag..." - Line 1281:
"# Issue mode"
docs/guides/local-quickstart.md — The PR updates 4 lines but misses:
- Line 79: Section header still says
"Option A: Local pipeline (prompt-driven, no GitHub interaction)" - Line 160:
"Local pipelines create PRs during the PR phase but do not interact with GitHub issues."
docs/architecture/orchestrator.md — The PR fixes 2 lines but misses:
- Line 128:
"issue number for issue-mode, pipeline ID for local-mode" - Lines 258-286: Entire section
"### 1. Local Mode (Interactive)"withEGG_ORCHESTRATOR_MODE=local
docs/hitl-decisions.md — Not touched at all, has 5 references:
- Line 16:
"In local mode, decisions carry a decision_type field..." - Line 155:
"In local mode, the orchestrator handles phase approval..." - Lines 211-225: Entire section
"## Local Mode: Type-Aware Terminal Rendering"
docs/development/TEST_COVERAGE_PLAN.md and docs/development/STRUCTURE.md — Multiple references to local_pipeline/ test directory and "local orchestrator integration tests." These are less critical since they reference actual directory/file names that may still exist, but the descriptive comments should be updated.
Non-blocking suggestions
-
docs/architecture/orchestrator.mdline 411 (Pipeline model docstring reference): Thenetwork_modefield description in the Pipeline model still says"None (auto from pipeline mode)". The actual behavior isNonedefaults to"public"— the doc update on line 91 is correct but there may be a stale docstring in the model code itself worth updating in a follow-up. -
The PR title includes
[doc-updater]suffix — this appears to be a bot-generated doc update. If this is automated, the tooling should be improved to catch all references, not just a subset.
Verdict
Request changes. The changes made are correct, but the PR needs to update all stale references to local/issue mode terminology across the docs, not just a few files. Shipping a partial rename creates inconsistent documentation.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-Review: Three issues remain
The new commit addresses most of the stale references identified in the previous review. The terminology updates in sdlc-pipeline.md, local-quickstart.md, hitl-decisions.md, and parts of orchestrator.md/STRUCTURE.md/TEST_COVERAGE_PLAN.md are correct. The API example fix and anchor link update are accurate. However, three issues need attention.
Blocking
1. docs/development/TEST_COVERAGE_PLAN.md:353 — Missed stale reference (same class as previous review)
Line 44 was correctly updated to "# Orchestrator pipeline integration tests", but line 353 has an identical tree listing that still says:
├── local_pipeline/ # Local orchestrator integration tests
This is the same incomplete-fix pattern from the first review. Fix: update to # Orchestrator pipeline integration tests to match line 44.
2. docs/architecture/orchestrator.md — "Standalone Mode" rename creates new inconsistency
The section heading at line 258 was renamed from "Local Mode (Interactive)" to "Standalone Mode (Interactive)", but the rest of the document was not updated to match:
- Line 3:
"three deployment modes for egg: local, remote-single, and distributed"— still says "local" - Line 285:
EGG_ORCHESTRATOR_MODE=local— config value is stilllocal - Line 582: env var table says
Deployment mode (local, remote-single, distributed)— still says "local"
More importantly, this rename conflates two different concepts. The eliminated feature is "local pipeline mode" (pipeline triggering method: prompt-driven vs issue-driven). The deployment mode "Local" describes where the system runs (single machine vs distributed) — a completely separate concept that was not eliminated. The PR's own description says it's about removing "local pipeline mode" references, not renaming deployment topology terms.
Fix (pick one):
- (a) Revert the section heading back to "Local Mode (Interactive)" since it describes deployment topology, not pipeline session type. The deployment mode name was never eliminated.
- (b) If "Standalone" is intentional, update line 3, line 285's comment, and line 582 to say "standalone" consistently. The
EGG_ORCHESTRATOR_MODEconfig value in code would also need updating, which is out of scope for a docs-only PR and argues for option (a).
3. docs/development/STRUCTURE.md — Incorrect skills/ removal
The PR removes the skills/ directory from both the tree listing (line 17 on main) and the directory details table, but skills/ still exists in the repository on both main and the PR branch (contains run-workflow/). Removing documentation for an existing directory makes the docs factually incorrect. This change is also unrelated to the terminology update.
Fix: restore the skills/ entry in both the tree listing and directory details table.
Non-blocking
The scripts/ → shared/ row reorder in the STRUCTURE.md directory table is cosmetic and harmless but unrelated to this PR's scope. Consider keeping it to minimize diff noise, or leave it — no strong opinion.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1. TEST_COVERAGE_PLAN.md:353 - Update missed stale 'Local orchestrator integration tests' reference to match line 44 2. orchestrator.md:258 - Revert 'Standalone Mode' back to 'Local Mode' since it describes deployment topology, not the eliminated pipeline session type 3. STRUCTURE.md - Restore incorrectly removed skills/ directory entries in both tree listing and directory details table
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
No agent-mode design concerns. Documentation-only terminology update with no impact on agent workflows, prompts, or sandbox enforcement. The coordinator.md changes (removing phase-role validation docs and branch parameter) reflect upstream code removal and don't affect how agents are instructed or constrained.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-Review: All blocking issues resolved
The new commit addresses all three blocking issues from the previous review. Changes verified against the codebase.
Previous blocking issues — status
1. TEST_COVERAGE_PLAN.md:353 — Fixed. Both line 44 and line 353 now consistently say # Orchestrator pipeline integration tests.
2. orchestrator.md "Standalone Mode" rename — Fixed. The heading was reverted to "Local Mode (Interactive)", which is correct — it describes deployment topology (single-machine vs distributed), not pipeline session type. The deployment mode named "local" was never eliminated; only the "local pipeline session mode" was.
3. STRUCTURE.md skills/ removal — Fixed. The skills/ entry is restored in both the tree listing and the directory details table, matching the actual repository contents (skills/run-workflow/SKILL.md).
New changes verified
- API example (
sdlc-pipeline.md): Confirmed against the actual endpoint atorchestrator/routes/pipelines.py. The old{"issue_number": 123, "mode": "issue"}was stale — themodefield no longer exists. The new{"issue_number": 123, "repo": "owner/repo", "branch": "egg/issue-123"}matches the current API which requiresrepoandbranch(when issue-driven). - Anchor link (
hitl-decisions.md): The internal reference to#prompt-driven-mode-type-aware-terminal-renderingmatches the renamed heading. No external docs link to this anchor — all cross-references tohitl-decisions.mdfrom other files use file-level links only. - Terminology consistency: All pipeline session type references correctly updated — "local mode/pipeline" → "prompt-driven", "issue mode" → "issue-driven". Deployment topology references to "Local Mode" correctly left unchanged.
- Remaining
local_pipeline/directory name references in STRUCTURE.md and TEST_COVERAGE_PLAN.md are correct — they document actual filesystem paths. The descriptive comments beside them have been properly updated.
Non-blocking
The scripts/ → shared/ row reorder in the STRUCTURE.md directory table is still present. Harmless but unrelated to this PR's scope — same note as last review, no action needed.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
|
egg feedback addressed. View run logs 10 previous review(s) hidden. |
Update documentation to reflect elimination of the local pipeline session
mode from commit 3c7f187 (Eliminate local pipeline mode, #1073).
Changes prompted by the code removal:
orchestrator.md: Fix network mode auto-fallback description — thelocal → localcase no longer exists;Nonenow defaults topublicsdlc-pipeline.md: Remove stale"mode": "issue"from API example (themodefield was removed from the pipeline creation API); update"local-mode pipelines" to "prompt-driven pipelines"; simplify HITL
approval description that said "both issue and local modes"
STRUCTURE.md: Fix test file comment from "Unified local/issue modebehavior tests" to match the updated docstring in the test file itself
local-quickstart.md: Replace "local pipeline" with "prompt-drivenpipeline" since the gateway no longer has a distinct "local" session mode
Issue: none
Test plan:
"mode": "issue"in API exampleseliminated
local → localfallbackAuthored-by: egg