docs: note #2593 context-PR hook wiring in sdlc-pipeline arch doc [doc-updater] - #2609
Conversation
There was a problem hiding this comment.
No agent-mode design concerns. Docs-only update to docs/architecture/sdlc-pipeline.md describing the expanded #2593 wiring — no prompts, agent code, or model-invocation surface touched.
— Authored by egg
There was a problem hiding this comment.
Doc-only update; claims verified against code.
Verified accurate:
- The shared
_maybe_open_base_pr_for_plan_to_implementwrapper exists atorchestrator/routes/pipelines.py:10490and owns the CUSTOM-mode guard + swallow-all-exceptions semantics described in the surrounding paragraph. - Four call sites (matching "all plan→implement transition paths"):
- inline
_run_pipelineauto-advance —routes/pipelines.py:20447(the original #2548 path) advance_phaseREST/MCP —routes/phases.py:479- HITL-approval recovery in
start_pipeline—routes/pipelines.py:21319 - implement-entry backstop —
routes/pipelines.py:19181
- inline
- #2548, #2593, and #2599 are real PR/issue numbers and #2599 (
1f420f9) is the commit onmainthat landed the wiring. - The note is correctly placed inside the same
> Schema 1.1 (#2548)blockquote so the historical context stays attached to the schema bump it belongs to.
Non-blocking nit: "fully wired as of #2548 / #2593" is slightly imprecise — the wiring was incomplete between #2548 and #2593 (that gap is exactly what #2593 fixed). "Fully wired as of #2593 (gap from #2548 closed)" would be marginally more accurate, but the current phrasing reads fine as a present-tense statement of the current state and the parenthetical at the bottom explicitly calls out the gap. Not worth a re-spin.
No correctness, security, or robustness concerns. The PR touches only docs/architecture/sdlc-pipeline.md and the prose matches reality.
— 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.
|
Addressed the one actionable item from the review. Per-item disposition:
— 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 prior review is a single-line wording change in docs/architecture/sdlc-pipeline.md ("fully wired as of #2548 / #2593" → "fully wired as of #2593 (gap from #2548 closed)") addressing a prior nit. Doc prose only — nothing touches prompts, agent orientation, output structure, or sandboxing.
— Authored by egg
There was a problem hiding this comment.
Re-review of delta since 65bd363 — single-line wording tweak in docs/architecture/sdlc-pipeline.md:132 addressing the prior non-blocking nit. Approved.
Delta:
- > **The context-PR mechanism is fully wired as of #2548 / #2593.** After
+ > **The context-PR mechanism is fully wired as of #2593 (gap from #2548 closed).** After
Verified:
- The new phrasing matches the chronology accurately: #2548 introduced the schema/context-PR mechanism, but #2593 was the PR that closed the wiring gap across the four call sites. "Fully wired as of #2593" is the correct present-tense fact, and "(gap from #2548 closed)" preserves the historical pointer.
- The lede now reads consistently with the parenthetical at the bottom of the blockquote (
docs/architecture/sdlc-pipeline.md:143-146), which enumerates the four paths #2593 added. The previous phrasing "as of #2548 / #2593" suggested the mechanism was fully wired in either of those PRs, which was imprecise. - No other text in the blockquote or surrounding paragraphs was touched. The schema-1.1 framing, the context-branch flow description, and the stacked-PR reconciler note are all unchanged.
- No code, prompts, or agent surface affected — still docs-only.
No correctness, security, or robustness concerns. The actionable suggestion from the prior review was addressed faithfully.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
|
egg review completed. View run logs 5 previous review(s) hidden. |
Update documentation to reflect changes from 1f420f9 / #2599:
docs/architecture/sdlc-pipeline.md: The schema 1.1 note previously stated the context-PR mechanism was "fully wired as of 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". Issue Context PR silently not created for pipelines (regression of #2548 / PR #2578) #2593 discovered the hook was wired into only the inline_run_pipelineauto-advance path, leavingadvance_phaseREST/MCP, HITL-approval recovery, and the implement-entry backstop without it. Fix #2593: wire context-PR hook into all plan→implement transition paths #2599 completes the wiring via a shared_maybe_open_base_pr_for_plan_to_implementwrapper. The note now references both 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 and Context PR silently not created for pipelines (regression of #2548 / PR #2578) #2593 and lists the four call sites.Triggered by: 1f420f9 (Fix #2593: wire context-PR hook into all plan→implement transition paths (#2599))
Authored-by: egg