docs: document branch-divergence OVERSEER_ALERT detector [doc-updater] - #2300
Conversation
There was a problem hiding this comment.
Doc-only PR documenting the branch-divergence detector from #2290. Verified the new section against the implementation in orchestrator/routes/pipelines.py:9888-9970 and _branch_divergence_tick/_health_monitor_poll. Most claims check out, but the false-positive example is incorrect.
Issue: false-positive example would not actually trigger the detector
docs/guides/pipeline-health-monitoring.md:236:
False positives: An agent legitimately including a
(#NNNN)reference in a commit subject (e.g., "Implement fix for issue #2222") would trigger the detector.
The example subject Implement fix for issue #2222 does not contain parentheses around #2222. The regex is re.compile(r"\(#\d+\)") (pipelines.py:9889) — it requires the literal ( and ) characters. Quick check:
>>> import re
>>> re.search(r"\(#\d+\)", "Implement fix for issue #2222")
# None — no match, no alert
>>> re.search(r"\(#\d+\)", "Cleanup task (#2222)")
# <re.Match object> — match, would alertSo the example as written would never trigger a false positive, which defeats the purpose of the paragraph (helping operators recognize benign matches). This is the one part of the new section an operator is most likely to consult — getting the trigger wrong here is misleading.
A correct example would be something like "Add tests (#2222)" or "Reference issue (#2222)" — i.e., a subject that mimics GitHub's squash-merge format with parens. The implementation comment in the alert body itself says "an agent legitimately copied a (#NNNN) reference" (with parens), so the doc should mirror that exactly.
Suggested fix:
False positives: An agent legitimately including a
(#NNNN)literal (with parentheses) in a commit subject — e.g.,"Reference benchmark suite (#2222)"— would trigger the detector. The alert body explains the false-positive scenario and instructs that no action is required if the diff against main looks clean.
Non-blocking suggestion: mention --no-merges
pipelines.py:9959 passes --no-merges to git log. The docs list the command as git log --pretty=format:%H%x09%s and omit --no-merges. Minor — unlikely to matter unless someone wonders why a true merge commit on the branch wasn't flagged — but worth a one-word edit in the bullet for fidelity:
- If count >
BRANCH_DIVERGENCE_THRESHOLD(20),git log --no-merges --pretty=format:%H%x09%s— list non-merge subjects
Spot-checks that passed
- 30-second tick interval — confirmed at
_health_monitor_poll(..., interval: float = 30.0). - Threshold is exclusive (
>20, not>=20) — confirmed atif ahead <= threshold: return ahead, [](pipelines.py:9953). OVERSEER_ALERTmetadata usesanomaly_type: "branch-divergence"— confirmed at_publish_branch_divergence_alert.- "Set clears when the contamination window goes empty (including on transient git errors)" — confirmed; the implementation comment at
_branch_divergence_ticksays this explicitly, and it falls out of_check_branch_divergence_for_alertreturning(0, [])on git error. - "Detection latency: phase-boundary granularity, poll thread does not fetch" — matches the implementation comment verbatim; cross-referenced no
git fetchcall in_health_monitor_poll. - "Complementary to the primary gate in #2282" — #2282 is "gateway blocks bare
git rebase origin/main", which is the right citation per #2290's PR description.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Address review feedback on PR #2300: - The false-positive example used a bare "#2222" reference, but the detector regex (\(#\d+\)) requires literal parens — the example would never trigger. Replace with "Reference benchmark suite (#2222)" and call out the regex requirement explicitly. - Add --no-merges to the documented git log invocation to match the implementation in pipelines.py:9959. Authored-by: egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-reviewed the doc-only delta in c8c83ffb. Both feedback items from my prior review are addressed precisely.
Resolved
False-positive example fixed (docs/guides/pipeline-health-monitoring.md:236). The example now reads "Reference benchmark suite (#2222)" with literal parens, and the paragraph now explicitly states The regex (\(#\d+\)) requires the literal ( and ) characters, so a bare #2222 reference does not match. This matches the actual regex _BRANCH_DIVERGENCE_PR_RE = re.compile(r"\(#\d+\)") at pipelines.py:9889 and gives operators an example that would actually fire.
--no-merges added (pipeline-health-monitoring.md:227). Bullet 2 now reads git log --no-merges --pretty=format:%H%x09%s — list non-merge subjects, matching the implementation at pipelines.py:9956-9962.
Spot-checks re-verified
- Threshold (20, exclusive) —
pipelines.py:9888,9953(if ahead <= threshold: return ahead, []). - Regex literal —
pipelines.py:9889. --no-merges—pipelines.py:9959.- "Set clears when the contamination window goes empty (including on transient git errors)" —
pipelines.py:10092-10097flushes the dedupe set wheneveroffendersis empty, with the explicit comment that transient git errors surface asoffenders == []and intentionally flush. Matches the doc verbatim. - "Detection latency: phase-boundary granularity" — no
git fetchcall in_health_monitor_poll, consistent with the doc's claim. OVERSEER_ALERTanomaly_type: "branch-divergence"— confirmed at_publish_branch_divergence_alert.
No new issues. Doc-only change, content accurate against the implementation.
— Authored by egg
|
egg review completed. View run logs 3 previous review(s) hidden. |
Update pipeline health monitoring guide to document the branch-divergence
OVERSEER_ALERT detector introduced in #2290 (Part of #2224, PR 3/3).
branch-divergencerow to the Tier 1 Tripwire Rules table indocs/guides/pipeline-health-monitoring.mddetection mechanism, detection latency (phase-boundary granularity),
deduplication behavior, and false-positive guidance
The detector watches
origin/<pipeline_branch>for commits that areTriggered by: db5cf68
Issue: none
Test Plan
implementation in
orchestrator/routes/pipelines.pyAuthored-by: egg