docs: document post-consensus rebaseline config [doc-updater] - #2273
Conversation
There was a problem hiding this comment.
Approve — docs match the #2253 implementation
Verified the four claims in this PR against the merged implementation:
Defaults
post_consensus_iteration_budget_seconds = 3600— matchesorchestrator/models.py:380-392(Field(default=3600, ge=60, ...)).post_consensus_max_total_seconds = 14400— matchesorchestrator/models.py:393-403(Field(default=14400, ge=60, ...)).
"Must be ≥ post_consensus_iteration_budget_seconds"
- Enforced by the cross-field validator in
orchestrator/models.py:640-645. The doc tables in bothconcurrent-execution.md:39andsdlc-pipeline.md:1255correctly surface this constraint.
"Resets each time a producer issues a new CONSENSUS_PROPOSE (initial or NACK→re-propose)"
- Matches
orchestrator/routes/pipelines.py:11484-11496:_latest_proposal_tsis sampled at loop entry (line 11418), then re-read each iteration; whencurrent_proposal_ts > last_seen_proposal_ts,last_progress_atis rebaselined totime.monotonic(). The BRC tracker's proposal stream covers both initial and NACK→re-propose events, so the parenthetical inconcurrent-execution.md:38is accurate.
"Hard ceiling regardless of proposal churn" / "30-second intervals"
- Matches
pipelines.py:11409(post_timeout_poll_interval = 30) andpipelines.py:11424-11431(the absolute-cap break fires before the iteration-budget check, so propose churn cannot defeat it).
Minor consistency observation (non-blocking)
The longer description in concurrent-execution.md:38 includes the "(initial or NACK→re-propose)" parenthetical, but the shorter version in sdlc-pipeline.md:1254 omits it. Both are accurate; the asymmetry is reasonable since the SDLC pipeline guide is the brief reference and the concurrent-execution guide is the deep dive. Not worth a re-spin.
No blocking issues. Approving.
— 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.
There was a problem hiding this comment.
Approve — re-review confirms no new content
The only commit since my previous review at dd5ff03 is the merge commit 0c4d74b ("Merge remote-tracking branch 'origin/main' into HEAD"). The merge resolved cleanly with no conflict edits — git diff origin/main..HEAD still shows the same +5/-1 across the same two files (docs/guides/concurrent-execution.md, docs/guides/sdlc-pipeline.md) that I verified against the #2253 implementation last pass.
All four doc claims (defaults 3600 / 14400, the ≥ cross-field constraint, the per-iteration rebaseline on CONSENSUS_PROPOSE, the absolute cap regardless of churn, and the 30-second poll interval) remain accurate against orchestrator/models.py and orchestrator/routes/pipelines.py on current main.
No new issues. Approving.
— Authored by egg
|
egg review completed. View run logs 3 previous review(s) hidden. |
Update documentation to reflect changes from 2a5d7ec (#2253):
post_consensus_iteration_budget_secondsandpost_consensus_max_total_secondsto the config tables indocs/guides/concurrent-execution.mdanddocs/guides/sdlc-pipeline.mdCONSENSUS_PROPOSE) and the absolute cap that bounds total waitTriggered by: #2253
Authored-by: egg