fix(tasks): stop reindex from writing defaults over indexed columns

[oranjeai]

Problem

TaskManager.reindex() passes eight of upsert_task()'s nine parameters, but
that method replaces the whole row (ON CONFLICT(id) DO UPDATE SET assigns every
column unconditionally), so each omitted or mis-keyed argument is written back as
its signature default. One reindex():

• wipes tags (no tags= kwarg), emptying the column every tag reader
  filters on: list_tasks, count_tasks and all three search_tasks tag paths
  share one ',' || tags || ',' LIKE '%,<tag>,%' predicate.
• resets status to the directory default, since nothing writes a
  **Status:** line. Unlike tags this is unrecoverable, the value existing only
  in the database: here, 392 of 395 active rows, all routing state. Reading that
  field is wrong regardless: the 20 files carrying one mostly hold prose, stored
  verbatim as the status.
• writes the URL into source: it reads the frontmatter key holding the
  source URL, while source is a vocabulary column.

Same class as #238, which fixed the completion paths and left reindex() as the
last such caller. It has no callers today, but is the only index-rebuild path and
two migrations name it as how FTS content gets filled.

Fix

Supply the three values at the call site with the idioms the six correct callers
use, leaving upsert_task() unchanged. status comes from the stored row, the
directory deciding terminality: a done/ file is terminal, and a stored done
on an active/ file is the orphan state docs/tasks.md forbids. tags
takes the file whenever the field is present, so a present-but-empty one is an
explicit clear, matching task_write; source_url and deadline take any
non-empty file value and otherwise keep the row.

parse_task_frontmatter is fixed in the same change. Its value pattern used
\s*(.+), whose \s* crosses a newline, so a field written blank captured the
next non-empty line instead of parsing as empty; it is line-bounded now. That
parser has three callers, so the change is validated against the full suite
below.

Verification

33 new tests, one behaviour per column: unfixed 23 fail, 10 pass as controls; all
33 pass fixed. Full suite 7 failed, 2966 passed, the same 7 failures by name as
base (pre-existing, unrelated), +33 passes. Sixteen mutants each weakening one
clause are all killed.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[oranjeai]

Pre-PR validation (a-i)

#	Question	Answer
a	Deterministic repro?	Yes. A tmpdir workspace + temp DB, one task file with **Tags:** ci,p2 / **Source:** <url> / **Deadline:** 2026-09-01 and a fully-populated row, then one reindex(). At base it reports CHANGED COLUMNS: ['tags', 'source', 'status'] on every run; with the fix, CHANGED COLUMNS: []. Not probabilistic. The parser half is equally deterministic: parse_task_frontmatter("# T\n\n**Tags:** \n\nBody text here.\n") returns {'tags': 'Body text here.'} at base and {'tags': ''} with the fix.
b	Root cause explained?	upsert_task() is a full-row replace (ON CONFLICT(id) DO UPDATE SET assigns status, source, source_url, deadline, tags from excluded.* unconditionally), and reindex() treats it as a partial update, passing 8 of its 9 parameters. The 3 it omits or mis-keys are written back as signature defaults, and a 4th (deadline) is nulled whenever the file lacks the line. Separately, parse_task_frontmatter's value pattern \s*(.+) crossed newlines and demanded a character, so a blank field captured the next non-empty line instead of parsing as empty.
c	Fix matches root cause?	Yes: the missing values are supplied at the offending call site, with upsert_task()'s replacement semantics deliberately untouched (the design #238 stated), and the parser's value is made line-bounded and optional so a blank field means what it says. No widened bound, no defensive guard, no disabled behaviour. Three alternatives were considered and rejected: making upsert_task() a partial update (changes shared semantics relied on by 6 correct callers), deleting reindex() as dead code (it is the only index-rebuild path), and fixing tags alone (leaves the strictly worse status reset in the same call).
d	Test intent preserved / new tests added?	No existing test was weakened or removed. 33 tests added, one behaviour per affected column and per blank-field position. reindex()'s primary purpose is itself pinned: it must still index unseen files with the directory default and still re-sync the title from the H1. Every preservation test additionally pins a file-derived value, because reindex logs and continues past a per-file exception, so a total no-op would otherwise satisfy "the value did not change".
e	Demonstrated in both directions?	Yes, same fixtures and same command. Base (exported from 94406ea and asserted to carry the crossing regex and no tags= in reindex): 23 failed, 10 passed -- the 10 are controls asserting unchanged behaviour. With the fix: 33 passed. Every defect has at least one test failing at base. Each arm reads its variant out of its own export and fails loudly on a label/variant mismatch, so no arm can silently run the wrong tree.
f	General across code paths?	All 7 production upsert_task() call sites were enumerated; the other 6 already pass tags=, so reindex() was the last caller treating a full replace as a partial update. mark_done() in the same file was verified safe. The FTS mirror derives fts_content from the same tags argument, so it is restored by the same change and pinned by its own test using a token that exists only in the stored tags column. The parser fix reaches all 3 of its callers (tasks/manager.py, gateway/routes/tasks.py, agent/tools/handlers/tasks.py), which is why it is validated against the whole suite rather than argued. routes/tasks.py carries a different (canonicalization) defect and is deliberately left to the task that owns it.
g	Generalizes across inputs?	Tags shapes measured and covered: mixed-case display text with spaces, comma form, [] empty array, whitespace-only, blank with no trailing space, absent field, absent field with no stored row. Blank-field positions covered: mid-file followed by body, mid-file followed by another metadata line, and end of file -- only the mid-file ones distinguish a line-bounded parse from a crossing one. Crossed with: both directories; new row and pre-existing row; each of tags/source/deadline present, blank and absent; nonterminal, terminal and orphan statuses; and **Status:** values that are prose as well as ones that are exactly a configured status.
h	Backward compatible?	No schema, migration, API or config change. The behaviour changes are what a future reindex() writes and how a blank frontmatter field parses; both preserve existing data, so they are strictly more conservative than the current code. A blank field previously yielded the next line's text, which no caller can have wanted. The one new overwrite is the orphan-status repair, which restores a documented invariant rather than introducing behaviour.
i	Invariants and contracts preserved?	upsert_task()'s contract is unchanged; it simply receives complete arguments. parse_task_frontmatter's contract is preserved for every non-blank field, verified by the suite: a normal value, a value with internal spaces and an absent field all behave exactly as before. created_at is never assigned by ON CONFLICT and is asserted unchanged. escalation_level and last_reminded_at are absent from the statement, so reindex() cannot touch them. The coupling that a file in done/ carries the terminal status, and the no-orphan invariant in docs/tasks.md, are both now enforced instead of violated. reindex() is awaited sequentially in one method, so there is no concurrency surface, no serialization format and no crash-recovery reader; the per-file try/except is unchanged and the added row read sits inside it.

Coverage is pinned rather than asserted: sixteen mutants, each weakening exactly one clause of the change, are all killed by at least one test, with an unmutated control green before and after the matrix. Each of the 10 base-passing control tests is killed by at least one mutant, so no shipped test is unpinned. Every mutant asserts its anchor matched exactly once, that the anchor is gone afterwards and that the file still parses, before any verdict is recorded. The mutant that skips every row before it is upserted kills all 26 reindex tests; its 7 survivors are the parser tests, which never call reindex.

No-regression proof by name rather than by count: base 7 failed, 2933 passed, fixed 7 failed, 2966 passed, with byte-identical sorted FAILED-name sets (cmp -s) and exactly +33 passes. That comparison is the evidence for widening the shared parser. The 7 pre-existing failures are 6 in an LLM-dependent memory date sweep and 1 timezone assertion; neither file references this code.

[oranjeai]

Internal second-model review - 2 rounds, 14 findings, 0 disagreed (click to expand)

Before opening this PR I reviewed it in two rounds: an independent cold read of the
resulting code, plus an independent model reviewing the diff against the PR contract.
Every finding below was accepted and fixed; none was overridden. Round 2 came back with
0 findings on the code.

#	Round	Raised by	Severity	Finding	Verdict
1	1	gate	blocker	parse_task_frontmatter's \s*(.+) crosses the newline, so a blank field captured the next non-empty line. 84 of 1311 live task files parse differently; reindex would have stored titles and headings as tags, the opposite of the explicit clear the code claimed. The shipped test missed it because its fixture put the blank field at EOF, the one position where the capture cannot reach further.	AGREE, fixed
2	1	gate	major	Both source tests used one constant for the file and the seeded row, so a mutant reducing fields.get("source") or stored to stored survived the whole suite.	AGREE, fixed
3	1	gate	major	reindex swallows per-file exceptions, so a mutant that raised before any upsert left all six preservation tests green: "value unchanged" is satisfied by a total no-op.	AGREE, fixed
4	1	own	major	The docstring said tags/source_url/deadline all take the file "when present"; only tags uses presence, the other two use truthiness.	AGREE, fixed
5	1	own	major	"eight of upsert_task()'s eleven columns" mixed a parameter count with a column count of a different set.	AGREE, fixed
6	1	own	nit	"matching both full-content save paths" cannot hold: the two save paths disagree with each other on a present-but-empty field.	AGREE, fixed
7	1	own	nit	Census figures re-derived rather than relayed: 392 of 395 active rows, not 387 of 390.	AGREE, fixed
8	1	own	nit	No test pinned the present-but-empty shape for deadline or source_url.	AGREE, fixed by items 1-2
9	2	own	blocker	The PR description described only the call-site change and never mentioned nerve/tasks/models.py, the highest-blast-radius file in the diff (a shared parser with three production callers).	AGREE, fixed
10	2	own	major	Round 1 changed the diff's size and file set, falsifying four verification figures in the description (18 tests, 12/6 base split, 2951 passed, 12 mutants). True: 33, 23/10, 2966, 16. All understated coverage, but the commit message already carried the true figures, so two published surfaces disagreed.	AGREE, fixed
11	2	own	nit	"21 files carrying one **Status:** line" re-measured as 20 files / 25 occurrences.	AGREE, fixed
12	2	own	nit	The docstring justifies each per-column rule as "matching the save path that already writes it". True for tags and deadline, but neither save path reads source_url from the file at all, so that column has no save path to match. The rule itself is correct and strictly better than before (it stops nulling the column on 684 live rows); only the stated reason over-generalises. Comment-only, tracked as a follow-up rather than held here.	AGREE, disclosed
13	2	own	nit	The parser's key pattern stays unanchored and last-wins, so body prose quoting a field name can still override the real header. Measured: files mis-reading a consumed key go from 100 before this change to 17 after; newly broken by this change: 0; fixed: 83. The residual is owned separately.	DROPPED, not this PR
14	2	own	nit	The file-first source_url rule stores 56 live values that are a URL plus a trailing annotation into a column read by exact match. Not introduced here: the previous code stores the identical 56, and differs on 684 rows where this change is better on all 684 and worse on none.	DROPPED, pre-existing

Two residuals are disclosed rather than fixed: finding 12 (a comment-only wording issue in
a docstring block this PR owns, filed as a follow-up so it lands against merged main
instead of conflicting here) and finding 13 (a distinct parser mechanism with its own
owner, which this change measurably improves rather than worsens).

Gate spend across both rounds: 4 runs, $14.29.

[oranjeai]

cc @fallintoplace @constkolesnyak - could you review this? reindex() treats upsert_task() as a partial update, but it is a full-row replace, so every argument it omits or mis-keys is written back as a signature default: it empties tags, resets status to the directory default (unrecoverable, since nothing writes a **Status:** line) and writes the source URL into the source vocabulary column. Same class as #238, which fixed the completion paths and left reindex() as the last such caller. It also line-bounds parse_task_frontmatter's value, which previously let a blank field capture the next non-empty line.