Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
715 changes: 715 additions & 0 deletions .egg-state/brc-history/3288-implement-slice-1.json

Large diffs are not rendered by default.

727 changes: 727 additions & 0 deletions .egg-state/brc-history/3288-implement-slice-1.md

Large diffs are not rendered by default.

37 changes: 25 additions & 12 deletions orchestrator/routes/pipelines.py
Original file line number Diff line number Diff line change
Expand Up @@ -6760,8 +6760,9 @@ def _build_role_context(
)
elif role_value == "documenter":
lines.append(
f"Focus your documentation on changes from plan phase `{phase_obj.id}`. "
"The following tasks were implemented in this phase:\n"
"Document the current state of the code in the areas these tasks "
"touch — a snapshot of how the system works now, not a log of what "
"changed. The following tasks were implemented in this phase:\n"
)
else:
lines.append("The following tasks were implemented in this phase:\n")
Expand Down Expand Up @@ -14165,8 +14166,9 @@ def _build_producer_orientation(
"read the contract (`egg-contract show`) to understand what is "
"being implemented. Check existing documentation structure — "
"README files, doc directories, inline documentation patterns. "
"Identify which docs will need updating once the implementation "
"is complete. "
"Identify which docs describe the surfaces this work touches, so "
"you can fold the resulting state into them as a snapshot of "
"current behavior once the implementation is complete. "
"**You MUST propose** even when the slice warrants no doc "
"updates (pure refactor / test-only / internal-only with no "
"documented-surface impact): the BRC consensus blocks until "
Expand Down Expand Up @@ -14781,17 +14783,28 @@ def _build_agent_prompt(
elif role_value == "documenter":
lines.extend(
[
"Update documentation for the changes made by the CODER agent:",
"Document the CURRENT STATE of the code after this change. "
"Write as if the code has always worked this way — the "
"slice/pipeline machinery that produced the change does not "
"belong in the documentation:",
"",
"1. Review the changed files (available in handoff data or via git diff)",
"2. Update relevant documentation (READMEs, docstrings, API docs)",
"3. Add or update inline code comments where helpful",
"2. Update relevant documentation (READMEs, docstrings, API docs) so it "
"describes how the system works now",
"3. Add or update inline code comments where they clarify current behavior",
"4. Commit documentation changes with descriptive messages",
"",
"Focus on:",
"- Accurate descriptions of new features or changes",
"- Updated usage examples if APIs changed",
"- Clear explanation of any breaking changes",
"Write snapshots, not changelogs:",
"- Describe what the code does now, not what changed or when it changed.",
"- NEVER reference SDLC artifacts — slice numbers, TASK-N ids, phase or "
"HITL iteration numbers — in any doc, docstring, or inline comment you write.",
"- Include historical context (issue links, \"previously X\" rationale, "
"migration notes) ONLY when it is tangibly valuable to a reader of the "
"current system, and prefer rationale (\"why it is this way\") over "
"chronology (\"what it used to be / when it changed\").",
"- When updating an existing doc, fold the new state into the snapshot and "
"REMOVE now-stale ledger or historical entries rather than appending "
"another layer.",
"",
"When documenting third-party integrations or external APIs, use WebSearch "
"and WebFetch (when available) to verify current API signatures, link to "
Expand All @@ -14814,7 +14827,7 @@ def _build_agent_prompt(
"docstring contracts that drift.",
"2. Propose a no-op: `egg-orch consensus propose "
"--no-changes-needed --no-changes-reason '<concrete reason, "
"e.g. slice-3 is a pure decomposition: symbol moves between "
"e.g. a pure decomposition: symbol moves between "
"submodules, no surfaced API change; no README / docs/ / "
"docstring surface impacted>'`. No artifacts or commit-sha "
"are needed.",
Expand Down
119 changes: 116 additions & 3 deletions orchestrator/tests/test_pipeline_prompts.py
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,9 @@ def test_documenter_with_phase_obj_includes_tasks(self):
)
assert "Phase Scope" in result
assert "TASK-2-1" in result
assert "Focus your documentation" in result
# Snapshot framing: documents the current state, not "the changes".
assert "current state" in result.lower()
assert "Focus your documentation on changes from plan phase" not in result

def test_tester_with_all_phases_shows_other_phases(self):
"""Tester sees other phases listed for orientation."""
Expand Down Expand Up @@ -1328,11 +1330,20 @@ def test_tester_only_current_phase_in_all_phases(self):
assert "Other Phases" not in result

def test_documenter_phase_intro_text(self):
"""Documenter gets documentation-focused intro text."""
"""Documenter intro frames the job as a current-state snapshot.

The per-phase summary must describe documenting how the system works
now, not "the changes from plan phase <id>" — the old phase-id-keyed
changelog framing is gone (snapshot-not-ledger, #3288).
"""
task = self._make_task("t-1", "Add feature")
phase = self._make_phase(tasks=[task])
result = _build_role_context("documenter", "# Issue", phase_obj=phase)
assert "Focus your documentation" in result
lower = result.lower()
assert "current state" in lower
assert "snapshot" in lower
# The removed changelog framing must not linger.
assert "Focus your documentation on changes from plan phase" not in result

def test_non_tester_non_documenter_phase_intro(self):
"""Non-tester/non-documenter execution roles get generic phase intro."""
Expand Down Expand Up @@ -6859,3 +6870,105 @@ def test_iteration_summary_from_live_tracker(self):
assert summary.verdict_matrix["reviewer_refine->refiner"] == ApprovalState.NACKED.value
assert summary.verdict_matrix["reviewer_agent_design->refiner"] == ApprovalState.ACKED.value
assert any("missing planner sections" in r for r in summary.nack_reasons)


class TestDocumenterSnapshotFraming:
"""The documenter agent is framed as a snapshot author, not a changelog
author (#3288).

The documenter's ``## Your Task`` block and producer orientation must
instruct current-state (snapshot) documentation, forbid SDLC artifacts
(slice numbers, ``TASK-N`` ids, phase/HITL iteration numbers) in any doc/
docstring/comment it writes, and prefer rationale over chronology — while
preserving the ``--no-changes-needed`` no-op propose path unchanged. These
tests pin the contract so the framing cannot silently regress to the old
"document the changes" wording.
"""

def _documenter_implement_prompt(self) -> str:
return _build_agent_prompt(
role_value="documenter",
phase="implement",
pipeline_id="test-pipe",
pipeline_mode="issue",
prompt="Document the implementation.",
issue_number=3288,
)

def test_implement_prompt_instructs_current_state_snapshot(self):
"""The implement-phase block instructs current-state (snapshot) docs."""
prompt = self._documenter_implement_prompt()
lower = prompt.lower()
assert "current state" in lower
# Snapshot framing, explicitly contrasted with changelogs.
assert "snapshot" in lower
assert "changelog" in lower

def test_implement_prompt_forbids_sdlc_artifact_references(self):
"""The block forbids slice/TASK/phase/HITL references in written docs."""
prompt = self._documenter_implement_prompt()
lower = prompt.lower()
# The prohibition names each banned SDLC artifact class.
assert "sdlc artifact" in lower
assert "slice number" in lower
assert "task-n" in lower
assert "hitl" in lower
assert "phase" in lower
# And it is phrased as a hard prohibition, scoped to written docs.
assert "never reference" in lower
assert any(surface in lower for surface in ("docstring", "inline comment", "comment"))

def test_implement_prompt_prefers_rationale_over_chronology(self):
"""The block prefers rationale ('why') over chronology ('what changed')."""
prompt = self._documenter_implement_prompt()
lower = prompt.lower()
assert "rationale" in lower
assert "chronology" in lower
# Folds new state into the snapshot and removes stale ledger entries
# rather than appending another historical layer.
assert "fold" in lower
assert "stale" in lower

def test_implement_prompt_drops_change_oriented_framing(self):
"""No stale assertion or instruction references the removed strings."""
prompt = self._documenter_implement_prompt()
assert "Update documentation for the changes made by the CODER agent" not in prompt
assert "Clear explanation of any breaking changes" not in prompt

def test_implement_prompt_preserves_no_op_propose_path(self):
"""The ``--no-changes-needed`` no-op propose path is still present and
its example reason no longer references a slice (#3027 path intact)."""
prompt = self._documenter_implement_prompt()
assert "--no-changes-needed" in prompt
assert "### When the slice warrants no doc updates" in prompt
# The reworded no-op example must not reintroduce a slice id.
assert "slice-3 is a pure decomposition" not in prompt

def test_per_phase_summary_uses_snapshot_framing(self):
"""The per-phase documenter summary describes current state, not the
'changes from plan phase <id>' changelog framing."""
phase = MagicMock()
phase.id = "phase-1"
phase.name = "Core"
phase.status = "in_progress"
task = MagicMock()
task.id = "t-1"
task.description = "Add feature"
task.files_affected = None
task.acceptance_criteria = None
task.role = "documenter"
phase.tasks = [task]

result = _build_role_context("documenter", "# Issue", issue_number=1, phase_obj=phase)
lower = result.lower()
assert "current state" in lower
assert "snapshot" in lower
assert "Focus your documentation on changes from plan phase" not in result

def test_orientation_frames_docs_as_current_state_snapshot(self):
"""Producer orientation frames doc work as folding state into a
current-behavior snapshot, not a list of 'docs that need updating'."""
orient = _build_producer_orientation("documenter", "implement", [])
lower = orient.lower()
assert "snapshot" in lower
assert "current behavior" in lower
12 changes: 8 additions & 4 deletions shared/egg_contracts/agent_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,17 @@ def depends_on(self, other: AgentRole) -> bool:

DOCUMENTER_ROLE = AgentRoleDefinition(
role=AgentRole.DOCUMENTER,
description="Updates documentation for the changes",
description="Documents the current state of the code",
category=AgentCategory.EXECUTION,
responsibilities=[
"Read the list of changed files from coder",
"Update relevant documentation",
"Add or update API documentation",
"Ensure README files are current",
"Describe how the code works now — a snapshot of the current "
"state, not a log of what changed or when",
"Never embed SDLC artifacts (slice numbers, TASK-N ids, phase or "
"HITL iteration numbers) in docs, docstrings, or comments",
"Prefer rationale (why it is this way) over chronology; fold new "
"state into the snapshot and remove now-stale ledger entries",
"Keep README and API documentation current",
],
dependencies=[AgentRole.CODER], # Must wait for coder
file_access=FileAccessPattern(
Expand Down
79 changes: 79 additions & 0 deletions shared/egg_contracts/tests/test_agent_roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
_PHASE_REVIEWERS,
AGENT_ROLE_TO_CONTRACT_ROLE,
AGENT_ROLES,
DOCUMENTER_ROLE,
EGG_ONLY_REVIEWERS,
REVIEWER_CODE_ROLE,
AgentCategory,
Expand Down Expand Up @@ -141,3 +142,81 @@ class TestNewLensReviewersNotEggOnly:
def test_not_in_egg_only_set(self) -> None:
assert AgentRole.REVIEWER_SECURITY not in EGG_ONLY_REVIEWERS
assert AgentRole.REVIEWER_CONCURRENCY not in EGG_ONLY_REVIEWERS


class TestDocumenterRoleSnapshotFraming:
"""``DOCUMENTER_ROLE`` carries snapshot-not-ledger framing while its gateway
write boundaries stay byte-identical (#3288).

The role description/responsibilities were reframed from "documentation for
the changes" to current-state documentation. That wording change MUST NOT
touch the ``FileAccessPattern`` — ``allowed_write`` / ``blocked_write`` are
a hard gateway constraint, so these tests pin both lists exactly.
"""

# Gateway write boundaries — must remain byte-identical after the wording
# change. The blocked_write list keeps code/test/.github surfaces out of
# the documenter's reach.
_EXPECTED_ALLOWED_WRITE = [
"docs/",
"**/README.md",
"**/*.md",
".egg-state/agent-outputs/",
]
_EXPECTED_BLOCKED_WRITE = [
"**/*.py",
"**/*.ts",
"**/*.tsx",
"**/*.js",
"**/*.jsx",
"**/*.go",
"**/*.java",
"tests/",
".egg-state/contracts/",
".github/",
]

def test_allowed_write_boundaries_unchanged(self) -> None:
assert DOCUMENTER_ROLE.file_access.allowed_write == self._EXPECTED_ALLOWED_WRITE

def test_blocked_write_boundaries_unchanged(self) -> None:
assert DOCUMENTER_ROLE.file_access.blocked_write == self._EXPECTED_BLOCKED_WRITE

def test_lookup_helper_returns_same_boundaries(self) -> None:
"""The registry lookup returns the same boundary lists."""
defn = get_role_definition(AgentRole.DOCUMENTER)
assert defn.file_access.allowed_write == self._EXPECTED_ALLOWED_WRITE
assert defn.file_access.blocked_write == self._EXPECTED_BLOCKED_WRITE

def test_documenter_cannot_write_code_or_tests(self) -> None:
"""Behavioral check on the boundaries: code/test paths stay blocked,
markdown stays writable."""
fa = DOCUMENTER_ROLE.file_access
assert fa.can_write("orchestrator/routes/pipelines.py") is False
assert fa.can_write("shared/egg_contracts/tests/test_agent_roles.py") is False
assert fa.can_write(".github/PULL_REQUEST_TEMPLATE.md") is False
assert fa.can_write("docs/architecture/brc-memory.md") is True
assert fa.can_write("orchestrator/README.md") is True

def test_role_metadata_stable(self) -> None:
"""Role identity, category, and coder dependency are unchanged."""
assert DOCUMENTER_ROLE.role == AgentRole.DOCUMENTER
assert DOCUMENTER_ROLE.category == AgentCategory.EXECUTION
assert AgentRole.CODER in DOCUMENTER_ROLE.dependencies

def test_description_uses_snapshot_framing(self) -> None:
"""Description expresses current-state documentation, not 'the changes'."""
description = DOCUMENTER_ROLE.description.lower()
assert "current state" in description
assert "for the changes" not in description

def test_responsibilities_forbid_sdlc_artifacts(self) -> None:
"""Responsibilities carry the snapshot rule and the no-SDLC-artifact
prohibition (slice/TASK/phase/HITL ids out of docs)."""
joined = " ".join(DOCUMENTER_ROLE.responsibilities).lower()
assert "snapshot" in joined
assert "sdlc artifact" in joined
assert "slice number" in joined
assert "task-n" in joined
# Prefers rationale over chronology.
assert "rationale" in joined
Loading