mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(gateway): i_documented auto-records journal:reflect from submission
The reflect gate's first, by-design tracing_gap ('journal:reflect missing')
counts toward the per-verb circuit breaker (i_documented limit 3 / 60s). A
documenter that fumbled note(scope='reflect') even twice got locked out and
went idle, stranding the task in awaiting_documentation with the PR orphaned.
i_documented now synthesizes the required reflect entry from the notes + files
it already carries, but only when the submission clears the notes/files gate
thresholds and the agent did not journal one themselves (theirs is richer and
left untouched). One call, no loop.
This commit is contained in:
@@ -308,6 +308,48 @@ class DocMixin(_Base):
|
||||
verb="i_documented",
|
||||
)
|
||||
|
||||
async def _ensure_doc_reflect(
|
||||
self,
|
||||
doc_agent_id: UUID,
|
||||
task_id: UUID,
|
||||
notes: str,
|
||||
files: list[str],
|
||||
) -> None:
|
||||
"""Record the journal:reflect i_documented requires, synthesized from
|
||||
the documenter's own submission, when they didn't journal one.
|
||||
|
||||
``i_documented`` requires a journal:reflect entry (pre-gateway parity,
|
||||
VERB_REQUIREMENTS). Documenters that never call note(scope='reflect')
|
||||
used to loop on the gate's tracing_gap until the per-verb circuit
|
||||
breaker (limit 3 / 60s) locked them out, stranding the task in
|
||||
awaiting_documentation. The ``notes`` + ``files`` this verb already
|
||||
carries ARE the reflection's substance, so we write the entry from
|
||||
them — one call, no loop.
|
||||
|
||||
Synthesis is skipped when (a) the agent already authored a reflect
|
||||
(theirs is richer — never clobber it) or (b) the submission is below
|
||||
the notes/files gate thresholds, so we never persist a reflect built
|
||||
from input ``_check_doc_gates`` will reject anyway; the agent retries
|
||||
with a real submission and we synthesize then.
|
||||
"""
|
||||
if await self.journal.has_reflect_for_task(doc_agent_id, task_id):
|
||||
return
|
||||
if len(notes.strip()) < settings.docs_notes_min_chars or not files:
|
||||
return
|
||||
content = (
|
||||
f"## What Done\n{notes.strip()}\n\n"
|
||||
f"Documented files: {', '.join(files)}\n\n"
|
||||
"## Next Steps\n- Hand off to PM review"
|
||||
)
|
||||
title = notes.strip().split("\n", 1)[0][:200]
|
||||
await self.journal.write_entry(
|
||||
agent_id=doc_agent_id,
|
||||
task_id=task_id,
|
||||
scope="reflect",
|
||||
title=title,
|
||||
content=content,
|
||||
)
|
||||
|
||||
async def _i_documented_spec_gate(
|
||||
self,
|
||||
doc_agent_id: UUID,
|
||||
@@ -400,6 +442,11 @@ class DocMixin(_Base):
|
||||
if spec_rejection is not None:
|
||||
return spec_rejection
|
||||
|
||||
# Satisfy the journal:reflect requirement from this submission's
|
||||
# notes + files when the documenter didn't journal one themselves —
|
||||
# otherwise the gate's tracing_gap loops into the circuit breaker.
|
||||
await self._ensure_doc_reflect(doc_agent_id, task_id, notes, files)
|
||||
|
||||
gate_rejection = await self._check_doc_gates(
|
||||
doc_agent_id, task_id, notes, files, owned_task
|
||||
)
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
"""i_documented synthesizes the required journal:reflect from the documenter's
|
||||
own submission so the reflect gate passes in one call.
|
||||
|
||||
Before this, i_documented returned a by-design tracing_gap ("journal:reflect
|
||||
missing") on the first call. That rejection counts toward the per-verb circuit
|
||||
breaker (limit 3 / 60s), so a documenter that fumbled note(scope='reflect')
|
||||
even twice got locked out and went idle, stranding the task in
|
||||
awaiting_documentation. The notes + files i_documented already carries are the
|
||||
reflection's substance, so _ensure_doc_reflect writes the entry from them —
|
||||
but only when the submission is substantive and the agent didn't journal one.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
|
||||
|
||||
|
||||
def _make_deps(**overrides: Any) -> ChoreographerDeps:
|
||||
base: dict[str, Any] = {
|
||||
"task": AsyncMock(),
|
||||
"work_session": AsyncMock(),
|
||||
"git": AsyncMock(),
|
||||
"a2a": AsyncMock(),
|
||||
"journal": AsyncMock(),
|
||||
"audit": AsyncMock(),
|
||||
"evidence_repo": AsyncMock(),
|
||||
"messaging": AsyncMock(),
|
||||
}
|
||||
base.update(overrides)
|
||||
return ChoreographerDeps(**base)
|
||||
|
||||
|
||||
_ADEQUATE_NOTES = "Documented the auth flow in README and the API reference."
|
||||
_SHORT_NOTES = "done" # below docs_notes_min_chars (20)
|
||||
_FILES = ["README.md", "docs/api.md"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_writes_reflect_from_submission_when_absent() -> None:
|
||||
journal = AsyncMock()
|
||||
journal.has_reflect_for_task.return_value = False
|
||||
c = Choreographer(_make_deps(journal=journal))
|
||||
doc_id, task_id = uuid4(), uuid4()
|
||||
|
||||
await c._ensure_doc_reflect(doc_id, task_id, _ADEQUATE_NOTES, _FILES)
|
||||
|
||||
journal.write_entry.assert_awaited_once()
|
||||
kwargs = journal.write_entry.await_args.kwargs
|
||||
assert kwargs["scope"] == "reflect"
|
||||
assert kwargs["agent_id"] == doc_id
|
||||
assert kwargs["task_id"] == task_id
|
||||
assert _ADEQUATE_NOTES in kwargs["content"]
|
||||
assert "README.md" in kwargs["content"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skips_when_agent_already_authored_reflect() -> None:
|
||||
journal = AsyncMock()
|
||||
journal.has_reflect_for_task.return_value = True
|
||||
c = Choreographer(_make_deps(journal=journal))
|
||||
|
||||
await c._ensure_doc_reflect(uuid4(), uuid4(), _ADEQUATE_NOTES, _FILES)
|
||||
|
||||
journal.write_entry.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skips_when_notes_below_threshold() -> None:
|
||||
journal = AsyncMock()
|
||||
journal.has_reflect_for_task.return_value = False
|
||||
c = Choreographer(_make_deps(journal=journal))
|
||||
|
||||
await c._ensure_doc_reflect(uuid4(), uuid4(), _SHORT_NOTES, _FILES)
|
||||
|
||||
journal.write_entry.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skips_when_no_files() -> None:
|
||||
journal = AsyncMock()
|
||||
journal.has_reflect_for_task.return_value = False
|
||||
c = Choreographer(_make_deps(journal=journal))
|
||||
|
||||
await c._ensure_doc_reflect(uuid4(), uuid4(), _ADEQUATE_NOTES, [])
|
||||
|
||||
journal.write_entry.assert_not_awaited()
|
||||
Reference in New Issue
Block a user