diff --git a/roboco/services/docs.py b/roboco/services/docs.py index 740822d3..10569bb8 100644 --- a/roboco/services/docs.py +++ b/roboco/services/docs.py @@ -100,6 +100,22 @@ _SIMILARITY_THRESHOLD = 0.75 _CONTENT_SUMMARY_LENGTH = 500 +def _coerce_doc_ref(d: object) -> DocRef: + """Build a DocRef from a stored ``Task.documents`` element. + + Canonical rows are dicts (``DocRef.model_dump()``). Defensive + against legacy/corrupted rows (#169): a bare path string is wrapped + instead of exploding ``DocRef(**str)`` and 500-ing the endpoint. + """ + if isinstance(d, DocRef): + return d + if isinstance(d, str): + return DocRef(path=d, title=Path(d).name, doc_type="doc") + if isinstance(d, dict): + return DocRef.model_validate(d) + raise TypeError(f"unsupported Task.documents element: {type(d).__name__}") + + # ============================================================================= # SERVICE # ============================================================================= @@ -417,7 +433,7 @@ class DocsService(BaseService): task = result.scalar_one_or_none() if not task: raise NotFoundError("Task", str(task_id)) - return [DocRef(**d) for d in (task.documents or [])] + return [_coerce_doc_ref(d) for d in (task.documents or [])] else: # Get agent's team and list files from filesystem team = get_agent_team(agent_id) @@ -526,8 +542,9 @@ class DocsService(BaseService): return None for doc in task.documents: - if doc.get("path") == path: - return DocRef(**doc) + ref = _coerce_doc_ref(doc) + if ref.path == path: + return ref return None async def _add_doc_to_task(self, task_id: UUID, doc_ref: DocRef) -> None: diff --git a/roboco/services/gateway/choreographer/doc.py b/roboco/services/gateway/choreographer/doc.py index 6f1bee4d..37eba732 100644 --- a/roboco/services/gateway/choreographer/doc.py +++ b/roboco/services/gateway/choreographer/doc.py @@ -32,12 +32,15 @@ requires. from __future__ import annotations import contextlib +from datetime import UTC, datetime +from pathlib import Path from types import SimpleNamespace from typing import TYPE_CHECKING, Any from roboco.config import settings from roboco.foundation.policy import lifecycle as spec_module from roboco.foundation.policy import tracing as _tr +from roboco.models.task import DocRef from roboco.services.gateway.envelope import Envelope from roboco.services.gateway.evidence_builder import build_evidence_for_task @@ -51,6 +54,30 @@ else: _Base = object +def _doc_refs_for(files: list[str], agent_id: UUID) -> list[dict[str, Any]]: + """DocRef-shaped dicts for ``Task.documents`` (a JSON column). + + ``i_documented`` receives a flat list of file paths, but + ``Task.documents`` is ``list[DocRef]`` persisted as dicts — readers do + ``DocRef(**d)`` / ``d["path"]`` and the doc indexer does ``d.get``. + Stamping bare strings 500s ``list_docs`` and breaks indexing (#169). + """ + now = datetime.now(UTC).isoformat() + slug = str(agent_id) + return [ + DocRef( + path=f, + title=Path(f).name, + doc_type="doc", + created_by=slug, + created_at=now, + updated_by=slug, + updated_at=now, + ).model_dump() + for f in files + ] + + def _extract_original_developer(task: Any) -> str | None: """Pull the original_developer slug out of a task's quick_context, if any. @@ -385,7 +412,7 @@ class DocMixin(_Base): # sees it. existing = await self.task.get(task_id) if existing is not None: - existing.documents = files + existing.documents = _doc_refs_for(files, doc_agent_id) await self.task.session.flush() runner = self._verb_runner() diff --git a/tests/unit/services/test_doc_refs_persistence.py b/tests/unit/services/test_doc_refs_persistence.py new file mode 100644 index 00000000..18a729d5 --- /dev/null +++ b/tests/unit/services/test_doc_refs_persistence.py @@ -0,0 +1,84 @@ +"""#169: i_documented must persist DocRef-shaped dicts, not bare strings. + +Smoke-15: be-doc i_documented(files=["README.md"]) → choreographer +doc.py stamped `existing.documents = files` (list[str]). Task.documents +is list[DocRef] persisted as dicts; readers do DocRef(**d) / d["path"] +and the indexer does d.get("path"). A bare string then 500'd +GET /docs (`TypeError: DocRef() argument after ** must be a mapping, +not str`) and would AttributeError the indexer. Fix: build proper +DocRef dicts at the source (_doc_refs_for) + defensively coerce on read +(_coerce_doc_ref) so legacy/corrupted rows don't explode. +""" + +from __future__ import annotations + +from uuid import uuid4 + +import pytest +from roboco.models.task import DocRef +from roboco.services.docs import _coerce_doc_ref +from roboco.services.gateway.choreographer.doc import _doc_refs_for + + +def test_doc_refs_for_builds_valid_docref_dicts() -> None: + """The stamped elements are dicts that survive DocRef(**d) and the + indexer's d.get('path') — the two paths that broke in smoke-15.""" + agent = uuid4() + inputs = ["README.md", "docs/api/endpoints.md"] + out = _doc_refs_for(inputs, agent) + + assert len(out) == len(inputs) + for d, expected_path, expected_title in ( + (out[0], "README.md", "README.md"), + (out[1], "docs/api/endpoints.md", "endpoints.md"), + ): + assert isinstance(d, dict), d + # Indexer path (roboco/services/task.py:_index_docs_background). + assert d.get("path") == expected_path + # list_docs / _get_existing_doc_ref path. + ref = DocRef(**d) + assert ref.path == expected_path + assert ref.title == expected_title + assert ref.doc_type == "doc" + assert ref.created_by == str(agent) + + +def test_doc_refs_for_empty_list() -> None: + assert _doc_refs_for([], uuid4()) == [] + + +def test_coerce_doc_ref_passthrough_docref() -> None: + ref = DocRef(path="a.md", title="a.md", doc_type="doc") + assert _coerce_doc_ref(ref) is ref + + +def test_coerce_doc_ref_from_dict() -> None: + ref = DocRef(path="a.md", title="a.md", doc_type="doc") + out = _coerce_doc_ref(ref.model_dump()) + assert isinstance(out, DocRef) + assert out.path == "a.md" + + +def test_coerce_doc_ref_from_bare_string_is_the_169_fix() -> None: + """The exact smoke-15 corruption: a bare path string must NOT raise + (previously `DocRef(**"README.md")` → TypeError → 500).""" + out = _coerce_doc_ref("README.md") + assert isinstance(out, DocRef) + assert out.path == "README.md" + assert out.title == "README.md" + assert out.doc_type == "doc" + + +def test_coerce_doc_ref_rejects_unsupported_type() -> None: + with pytest.raises(TypeError, match=r"unsupported Task\.documents element"): + _coerce_doc_ref(123) + + +def test_source_output_round_trips_through_read_coercion() -> None: + """End-to-end invariant: what _doc_refs_for writes is exactly what + _coerce_doc_ref reads back without loss.""" + agent = uuid4() + stamped = _doc_refs_for(["README.md"], agent) + refs = [_coerce_doc_ref(d) for d in stamped] + assert [r.path for r in refs] == ["README.md"] + assert refs[0].created_by == str(agent)