tasks: store the PATCH route's frontmatter tags in the canonical form - #263
Conversation
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 '%,<tag>,%'` (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.
|
|
Internal second-model review and reviewer adjudication (2 cold reviews + 1 independent gate run, 7 findings, all adjudicated)Before this PR was opened it went through my own cold code review plus an independent
1 (mine, AGREE). Both surfaces said "all four fail on 2 and 3 (mine, AGREE). The writer census was described as grepping 4 (gate, DISAGREE). The ask is a one-time data migration, or deployment-wide census
On the census itself: it is recorded -- 1325 live rows, 0 stored non-canonically, 0 5 (gate, DISAGREE). Both flagged docstrings record measurements rather than 6 and 7 (mine, second review round, AGREE). After the first round's fix landed I
Both overstated the rejected alternative's cost, so they were not merge hazards, and the Gate spend for this PR: $5.75 across 2 runs (1 approach gate, 1 code gate). |
Pre-PR validation gate (click to expand)
|
3 participants
Symptom
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 underalphabut notbetaorgamma, whilecount_tasks(tag='beta')is short by one. Noerror, and the markdown on disk stays correct.
Root cause
PATCH /api/tasks/{id}with acontentbody re-syncs the frontmatter into the SQLite row,and wrote the
**Tags:**line verbatim:The exact-tag predicate
',' || tags || ',' LIKE '%,<tag>,%'(nerve/db/tasks.py, sharedby
list_tasks,count_tasksand thesearch_taskstag filter) matches only when barecommas delimit the tag, so the leading space on every key after the first defeats it.
LIKEis ASCII-case-insensitive, so the missinglower()is the lesser half. Every otherwhole-value writer normalizes first; this one did not. UI path:
TaskDetailPage.saveTaskContent->api.updateTask(id, {content}).Fix
Call the existing normalizer here, as its two siblings (
task_write_handler,task_update_handler) do:One source file, +7/-2; the rest of the diff is tests. No schema, migration, settings,
API-shape or frontend change, and no data repair: a census of every live row here found
none stored non-canonically.
Tests
Four cases in
tests/test_db.py::TestPatchRouteTagCanonicalizationdrive the real route:the stored value is canonical; every key is independently findable via
list_tasks(tag=)and counted; an unsorted line with a duplicate is sorted and deduped;a canonical line is unchanged. Per-key assertions are load-bearing: a
first-key-only variant passes unfixed, as does one routed through
search_tasks'sexact-id strategy, which strips elements in Python.
Three fail on
main; the already-canonical case passes there by construction, since theunfixed route stores its input verbatim. All four pass here. Full suite 2937 passed,
with
main's same 7 pre-existing failure names.TaskManager.reindexerases the column outright (notags=). Separate fix.