From 21ad55c1f28be38990f5563f1ff614033362250f Mon Sep 17 00:00:00 2001 From: Oranje AI <293843428+oranjeai@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:48:40 +1200 Subject: [PATCH] tasks: store the PATCH task route's frontmatter tags canonically Editing a task in the web editor makes most of its tags unfindable. Save a task whose markdown carries `**Tags:** Alpha, beta, Gamma` and a tag-filtered listing finds it under `alpha` but not `beta` or `gamma`, while count_tasks(tag='beta') is short by one. No error is raised and the markdown on disk stays correct, so nothing surfaces the loss. PATCH /api/tasks/{id} with a `content` body re-syncs the markdown frontmatter into the SQLite row, and passed the parsed `**Tags:**` value into upsert_task verbatim, in display form. The exact-tag predicate is `',' || tags || ',' LIKE '%,,%'` (nerve/db/tasks.py, shared by list_tasks, count_tasks and the search_tasks tag filter), which matches only when bare commas delimit the tag -- so the leading space on every key after the first defeats it. SQLite LIKE is ASCII-case-insensitive, so the missing lower() is the lesser half of the same skew: it bites only non-ASCII keys. Every other whole-value writer normalizes first, so this call site was the only one storing a non-canonical value. Fix: call the project's existing normalizer at that call site, spelled as its two sibling writers spell it (task_write_handler, task_update_handler): tags_to_string(parse_tags_string(...)). Normalizing inside upsert_task instead was rejected -- it would widen one route to every task write in the process, and six of its seven call sites already pass a canonical value -- as was relaxing the SQL to tolerate spaces, which would leave two representations of one tag set in the column and have to be repeated at four predicate sites. Four tests drive the real route. The stored value is canonical; every key is independently findable via list_tasks(tag=) and counted by count_tasks(tag=); an unsorted line with a duplicate key is sorted and deduped; an already-canonical line is byte-unchanged. Per-key assertions are load-bearing and this was measured, not assumed: a first-key-only variant passes against the unfixed route, and search_tasks' exact-task-id strategy filters in Python via _row_matches_filters, which strips each element and therefore tolerates the display form too, so a test routed through either is vacuous. The sort-and-dedup case is likewise not decoration: without it, a hand-rolled `.replace(", ", ",").lower()` substitute passes every other assertion, because the fixture `Alpha, beta, Gamma` is already ordered and duplicate-free. Three fail on main and all four pass here; the already-canonical case passes on main too, by construction, since the unfixed route stores its input verbatim. Full suite: 2937 passed, with main's identical 7 pre-existing failures by name (6 in tests/test_memu_bridge.py, 1 in tests/test_telegram_sessions.py), none in a file this touches. No schema, migration, settings, API-shape or frontend change, and no data repair: a read-only census of every live task row on this instance found none stored non-canonically, so the defect was latent rather than already fired. TaskManager.reindex passes no tags= at all, so upsert_task's "" default erases the column on every row it indexes. That is a different mechanism in a different file and is fixed separately. --- nerve/gateway/routes/tasks.py | 9 +++- tests/test_db.py | 79 +++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/nerve/gateway/routes/tasks.py b/nerve/gateway/routes/tasks.py index 128febdc..adb2fae7 100644 --- a/nerve/gateway/routes/tasks.py +++ b/nerve/gateway/routes/tasks.py @@ -139,7 +139,12 @@ async def update_task(task_id: str, req: TaskUpdateRequest, user: dict = Depends file_path.write_text, req.content, encoding="utf-8", ) # Re-sync title from markdown to SQLite - from nerve.tasks.models import parse_task_title, parse_task_frontmatter + from nerve.tasks.models import ( + parse_task_frontmatter, + parse_task_title, + parse_tags_string, + tags_to_string, + ) new_title = parse_task_title(req.content) fields = parse_task_frontmatter(req.content) await deps.db.upsert_task( @@ -150,7 +155,7 @@ async def update_task(task_id: str, req: TaskUpdateRequest, user: dict = Depends source=task.get("source"), source_url=task.get("source_url"), deadline=fields.get("deadline") or task.get("deadline"), - tags=fields.get("tags") or task.get("tags", ""), + tags=tags_to_string(parse_tags_string(fields.get("tags") or task.get("tags", ""))), content=req.content, ) diff --git a/tests/test_db.py b/tests/test_db.py index 578985d7..b4b2134c 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -870,6 +870,85 @@ def test_roundtrip(self): assert tags_to_string(parse_tags_string(original)) == original +@pytest.mark.asyncio +class TestPatchRouteTagCanonicalization: + """``PATCH /api/tasks/{id}`` must store the frontmatter tags canonically. + + The route re-syncs the markdown's ``**Tags:**`` line into the row. Written + verbatim, the display form (``Alpha, beta``) leaves a leading space on every + key after the first, and the exact-tag predicate + ``',' || tags || ',' LIKE '%,,%'`` then matches only the first key, so + a tag set through the web editor is silently unfindable by tag-filtered + listing and counting. Both sibling writers (``task_create_handler``, + ``task_write_handler``) normalize through ``tags_to_string(parse_tags_string(...))``. + + Assertions must be PER KEY. A first-key-only variant passes against the + unfixed route, and ``search_tasks`` strategy 1 (exact task-id) filters in + Python via ``_row_matches_filters``, which strips each element and therefore + tolerates the display form, so a test routed through either is vacuous. + """ + + async def _patch(self, db, tmp_path, monkeypatch, content, initial_tags="old"): + """PATCH task ``t1``'s content through the real route, return its row.""" + from nerve import config as cfg + from nerve.config import NerveConfig + from nerve.gateway.routes import tasks as tasks_route + + ws = tmp_path / "ws" + rel = "memory/tasks/active/t1.md" + (ws / "memory" / "tasks" / "active").mkdir(parents=True, exist_ok=True) + (ws / rel).write_text(f"# T\n\n**Tags:** {initial_tags}\n\nbody\n", encoding="utf-8") + monkeypatch.setattr(cfg, "_config", NerveConfig(workspace=ws)) + await db.upsert_task( + task_id="t1", file_path=rel, title="T", status="pending", + tags=initial_tags, content="body", + ) + monkeypatch.setattr(tasks_route, "get_deps", lambda: type("D", (), {"db": db})()) + await tasks_route.update_task( + "t1", tasks_route.TaskUpdateRequest(content=content), user={}, + ) + return await db.get_task("t1") + + async def test_display_form_is_stored_canonically(self, db: Database, tmp_path, monkeypatch): + row = await self._patch( + db, tmp_path, monkeypatch, "# T\n\n**Tags:** Alpha, beta, Gamma\n\nbody\n", + ) + assert row["tags"] == "alpha,beta,gamma" + + async def test_every_key_is_findable_by_exact_tag_filter( + self, db: Database, tmp_path, monkeypatch, + ): + row = await self._patch( + db, tmp_path, monkeypatch, "# T\n\n**Tags:** Alpha, beta, Gamma\n\nbody\n", + ) + for tag in ("alpha", "beta", "gamma"): + found = [r["id"] for r in await db.list_tasks(tag=tag, status="all")] + assert "t1" in found, f"tag {tag!r} not findable; stored={row['tags']!r}" + assert await db.count_tasks(tag=tag, status="all") == 1 + + async def test_value_is_sorted_and_deduplicated( + self, db: Database, tmp_path, monkeypatch, + ): + """The stored form is the project normalizer's full output, not just + despaced and lowercased text. An already-sorted fixture cannot see this: + a hand-rolled ``.replace(", ", ",").lower()`` reproduces it exactly, and + would then diverge on any real tag line whose keys are out of order or + repeated. Ordering and dedup are what make the column comparable. + """ + row = await self._patch( + db, tmp_path, monkeypatch, "# T\n\n**Tags:** Gamma, beta, Alpha, beta\n\nbody\n", + ) + assert row["tags"] == "alpha,beta,gamma" + + async def test_already_canonical_value_is_unchanged( + self, db: Database, tmp_path, monkeypatch, + ): + row = await self._patch( + db, tmp_path, monkeypatch, "# T\n\n**Tags:** alpha,beta\n\nbody\n", + ) + assert row["tags"] == "alpha,beta" + + # --- Plan lifecycle --- @pytest.mark.asyncio