mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(content): move lifecycle-transition notes off quick_context into markers
The structure-everything sweep found four more writers packing key:value soup into quick_context (the human ResumptionNote field), same anti-pattern as the already-fixed approve_and_start_notes: - _record_completion_notes -> completion_notes:<text> - escalate_to_ceo -> escalation_notes:<text> - ceo_approve -> ceo_approval_notes:<text> - ceo_reject -> ceo_rejection:<reason> Route them through a unified orchestration_markers['transition_notes'] dict (keyed by event) via markers.set_transition_note, so quick_context carries only the structured ResumptionNote and the panel never shows raw <event>:<text> soup. Adds the typed accessor + a roundtrip test.
This commit is contained in:
@@ -171,3 +171,27 @@ def get_approve_and_start_notes(task: HasMarkers) -> str | None:
|
||||
|
||||
def set_approve_and_start_notes(task: HasMarkers, notes: str) -> None:
|
||||
set_marker(task, APPROVE_AND_START_NOTES, notes)
|
||||
|
||||
|
||||
# --- lifecycle transition notes -------------------------------------------- #
|
||||
# A PM/CEO note attached to a lifecycle transition (completion, escalate_to_ceo,
|
||||
# ceo_approval, ceo_rejection). These used to be string-packed into
|
||||
# ``quick_context`` as ``<event>:<text>`` soup; they live here keyed by event so
|
||||
# ``quick_context`` carries only the human ResumptionNote.
|
||||
|
||||
TRANSITION_NOTES = "transition_notes"
|
||||
|
||||
|
||||
def get_transition_note(task: HasMarkers, event: str) -> str | None:
|
||||
notes = get_marker(task, TRANSITION_NOTES)
|
||||
if not isinstance(notes, dict):
|
||||
return None
|
||||
val = notes.get(event)
|
||||
return str(val) if val else None
|
||||
|
||||
|
||||
def set_transition_note(task: HasMarkers, event: str, note: str) -> None:
|
||||
existing = get_marker(task, TRANSITION_NOTES)
|
||||
notes = dict(existing) if isinstance(existing, dict) else {}
|
||||
notes[event] = note
|
||||
set_marker(task, TRANSITION_NOTES, notes)
|
||||
|
||||
+9
-31
@@ -3873,13 +3873,9 @@ class TaskService(BaseService):
|
||||
"""Append completion_notes entry to quick_context when supplied."""
|
||||
if not notes:
|
||||
return
|
||||
existing_context = task.quick_context or ""
|
||||
note_entry = f"completion_notes:{notes}"
|
||||
task.quick_context = (
|
||||
f"{existing_context}\n{note_entry}".strip()
|
||||
if existing_context
|
||||
else note_entry
|
||||
)
|
||||
# A coordination annotation, not a human ResumptionNote — store as a
|
||||
# marker so quick_context never carries `completion_notes:<text>` soup.
|
||||
markers.set_transition_note(task, "completion", notes)
|
||||
|
||||
async def submit_for_pm_review(
|
||||
self,
|
||||
@@ -4267,15 +4263,9 @@ class TaskService(BaseService):
|
||||
)
|
||||
return None
|
||||
|
||||
# Store escalation notes
|
||||
# Store the escalation note as a marker, not quick_context soup.
|
||||
if notes:
|
||||
existing_context = task.quick_context or ""
|
||||
note_entry = f"escalation_notes:{notes}"
|
||||
task.quick_context = (
|
||||
f"{existing_context}\n{note_entry}".strip()
|
||||
if existing_context
|
||||
else note_entry
|
||||
)
|
||||
markers.set_transition_note(task, "escalate_to_ceo", notes)
|
||||
|
||||
# Validate transition with PM role requirement
|
||||
self._validate_and_set_status(
|
||||
@@ -4345,15 +4335,9 @@ class TaskService(BaseService):
|
||||
)
|
||||
return None
|
||||
|
||||
# Store CEO notes
|
||||
# Store the CEO's approval note as a marker, not quick_context soup.
|
||||
if notes:
|
||||
existing_context = task.quick_context or ""
|
||||
note_entry = f"ceo_approval_notes:{notes}"
|
||||
task.quick_context = (
|
||||
f"{existing_context}\n{note_entry}".strip()
|
||||
if existing_context
|
||||
else note_entry
|
||||
)
|
||||
markers.set_transition_note(task, "ceo_approval", notes)
|
||||
|
||||
task.completed_at = datetime.now(UTC)
|
||||
# Validate transition with CEO role requirement
|
||||
@@ -4548,14 +4532,8 @@ class TaskService(BaseService):
|
||||
)
|
||||
return None
|
||||
|
||||
# Store CEO rejection reason
|
||||
existing_context = task.quick_context or ""
|
||||
rejection_entry = f"ceo_rejection:{reason}"
|
||||
task.quick_context = (
|
||||
f"{existing_context}\n{rejection_entry}".strip()
|
||||
if existing_context
|
||||
else rejection_entry
|
||||
)
|
||||
# Store the CEO's rejection reason as a marker, not quick_context soup.
|
||||
markers.set_transition_note(task, "ceo_rejection", reason)
|
||||
|
||||
# A coordination/integration root (no repo of its own, carries a
|
||||
# product) has no developer to revise it — NEEDS_REVISION is
|
||||
|
||||
@@ -68,6 +68,17 @@ def test_approve_and_start_notes_roundtrip() -> None:
|
||||
assert m.get_approve_and_start_notes(t) == "Board approved; build it."
|
||||
|
||||
|
||||
def test_transition_note_roundtrip_keyed_by_event() -> None:
|
||||
t = _task()
|
||||
assert m.get_transition_note(t, "ceo_rejection") is None
|
||||
m.set_transition_note(t, "completion", "Reviewed and merged.")
|
||||
m.set_transition_note(t, "ceo_rejection", "Needs the migration first.")
|
||||
# Each event keeps its own note; setting one doesn't clobber another.
|
||||
assert m.get_transition_note(t, "completion") == "Reviewed and merged."
|
||||
assert m.get_transition_note(t, "ceo_rejection") == "Needs the migration first."
|
||||
assert m.get_transition_note(t, "never_set") is None
|
||||
|
||||
|
||||
def test_documenter_self_heal_head_supersede() -> None:
|
||||
t = _task()
|
||||
m.set_documenter(t, "doc-uuid")
|
||||
|
||||
Reference in New Issue
Block a user