fix: id error handling - #142
Merged
Merged
Conversation
`task edit` / `audit edit` / `research edit` re-parse on save, but the parse was tolerant of a malformed Crockford id, so an interactive edit could put one on disk that `lint` would later flag. The filename guard and `task set` both refused it; the editor path was the hole. acceptEdited wraps the parse closure for the editor path only. It is deliberately NOT folded into parseTask/parseAudit/parseResearch, which is the obvious-looking fix: reads must stay tolerant so `lint` can REPORT a bad id already on disk, while writes refuse to create one. Tightening the parser would turn every such file from "flagged and fixable" into "unreadable" — the same read/write asymmetry the filename-id guard already has. H2 of audit 2026-08-24-planning-state-vocabulary.
An unchecked box meant "not yet", "won't do", and "no longer applies" all at once,
so a task could sit at 3/7 forever with no way to say why the other four were not
coming. A criterion can now carry a state and a reason:
- [ ] Ship the migration · **deferred:** waiting on the schema ADR
Written as a checkbox PLUS a suffix rather than a new marker, so every criterion
ever authored still parses and no migration is needed — the states are additive
and a plain box keeps meaning exactly what it meant.
`task ac --defer/--wontfix/--tracked/--na <n> --reason <why>` writes them through
the same atomic body-replace path the checkbox flip uses. Shipping the write path
WITH the vocabulary is the point: a state reachable only by hand-editing is one
nobody can be held to, which is how the finding vocabulary drifted from its own
documentation (M3 of 2026-08-17-finding-status-surface).
The words that mean the same thing for a criterion and a finding — deferred,
wontfix, tracked — are spelled once in domain/resolution.go and shared. It is
modelled as an OVERLAP, not a subset: `met` is not a finding status and
`superseded` is not a criterion state, and claiming otherwise is exactly what
would let the two drift unnoticed.
…ns with the tool
The vocabulary had a word nobody used and lacked one everybody improvised.
`landed` had zero uses across the corpus. Meanwhile 7 of 13 `deferred` findings
were not deferrals at all but handoffs, written in prose as `deferred → tracked in
task X` by two authors months apart. `tracked` is now a real status: it counts
toward the audit's DONE band, because the audit's interest concludes when a finding
is transferred, and it REQUIRES a destination — a handoff with nowhere to follow is
the improvisation it replaces. It renders `→` rather than `✔`; a tick would claim
the work was finished here.
Also on the finding surface:
* `--note` writes the `**Resolution:**` paragraph — one paragraph, wrapped at 80
by rune count, placed inside the right finding by construction. Re-noting
replaces; an empty note removes it; a newline is refused, since one could open
a heading or a fence mid-finding.
* `--pr N` renders the canonical `(PR #N)`. `PR 12`, `pull/12`, and `(PR #12)`
read alike to a human and differently to grep.
* A leading ✅/⏳/⛔ is decoration, not the status. It was being captured AS the
status, which the 2026-08-17 audit measured as the single highest-frequency
cause of red audits. The span still covers the glyph so a re-stamp replaces it;
leaving `✅ deferred` would restate the two-lists-disagree bug in the file.
* A finding's section now ends at the next HEADING, not just the next finding.
Without that bound the last finding annexed `## Candidate tasks` and a note
appended to it landed under the wrong heading.
* Status and note given together are ONE atomic write. Two would leave a window
where a finding claims `fixed` with last round's explanation.
Lint gained the two rules a hand edit can still break: a destination-less
`tracked`, and a second `**Resolution:**` block whose text would be silently
ignored. The `schema audit` conventions line is now BUILT from FindingStatuses()
rather than transcribed, so the guidance cannot fall a word behind the code the way
the table in M3 did.
Closes M1, M2, M3 of 2026-08-17-finding-status-surface.
An audit's headline percent counted only fixed/tracked findings, so a fully
resolved audit could contradict itself on its own row:
✔ closed 2026-06-27-consumer-data-flow-architecture ██████▒▒ 77% fixed 17/22
Zero open findings, seventeen fixed, five deliberately dropped — resolved, closed,
and reading 77%. The old code documented the workaround rather than the bug: the
comment on AuditPercentLabel explained that "a fully-triaged audit is legitimately
0% fixed yet ready to close, so its number names its unit". The unit label was
compensating for a number measuring the wrong thing.
Percent is now the SETTLED share — every finding with a terminal disposition,
however it got there. Deciding not to fix a finding is resolving it. `Settled` is
already the domain's word for the boolean that drives "ready to close", so 100%
settled and `✔ ready to close` are one fact stated twice rather than two numbers to
reconcile. Nothing about HOW it settled is lost: the segmented bar still bands it.
Those bands were undocumented anywhere despite appearing on three surfaces, so the
vocabulary now lives in theme (BandDone/Active/Dropped/Open) with progressbar
deriving from it, the TUI audits legend names all four, and `audit list --help`
spells out the groupings. Seven statuses collapse into four bands deliberately — a
reader takes in the shape at a glance, and seven hues would be a decode task.
TestTallyFindings_EveryStatusLandsInABand pins the grouping as total. A status
added without a band fails nothing today: the counts just come up short and the bar
under-fills, which reads as work remaining.
Human surfaces only — audit percent is in no JSON envelope or column projection.
…ation
Four contract changes, three additive and one not:
1.45 a task's acceptance tally carries `explained` — how many UNMET criteria
say why. Zero for every task written before the vocabulary existed.
1.46 criteria may carry `state` and `reason`. Absent for a plain met/not-met
criterion, so an older body serialises unchanged.
1.47 the finding-status vocabulary drops `landed` and gains `tracked`. NOT
additive: a consumer switching on the status set must learn the new word.
No audit in the corpus ever used `landed`, so nothing in-tree breaks.
1.48 findings may carry `note` (the resolution paragraph) and
`status_decoration` — everything after the status word. The wire had been
dropping the date on `fixed 2026-08-24` and the destination on `tracked by
<id>` entirely, because `status` carries only the vocabulary token. The
`schema` contract also now publishes `criterion_states`, for the reason it
publishes `finding_statuses`: `state` has been a wire field since 1.46 with
no published vocabulary, so an agent had to trigger an error and parse
prose to learn the set.
The criterion-state descriptions in dto.go point at that contract rather than
transcribing the list — they had already fallen a word behind by omitting
`tracked`, which is M3 of 2026-08-17 recurring inside the change that closed it.
The README cheat sheet listed `audit findings` (the query) but never `audit finding` (the write), and described `task ac` as just `--check/--uncheck` — the binary vocabulary the states replaced. A reader would have concluded neither exists. Also regenerates the CLI reference, which gains tskflwctl_audit_finding.md and picks up the `audit list` band legend and the new `task ac` flags.
Three audits, all driven with the tool rather than hand-edited:
* 2026-08-24-planning-state-vocabulary — the adversarial multi-lens review of
the criterion vocabulary. 8/8, closed. Its H1 was a real corruption bug: an
offset desync in SetFindingStatus rewrote `## Candidate tasks` to
`## Candidafixedasks` on files containing code fences, while reporting success.
* 2026-08-24-finding-note-and-vocabulary-selfreview — a critical read of this
branch's own work. 9/10 fixed. The one left open, M3, is a decision rather
than a defect: a `tracked` criterion needs no destination while a `tracked`
finding does, and that asymmetry is currently accidental rather than stated.
* 2026-08-17-finding-status-surface — M1, M2, M3 closed, each carrying a
`**Resolution:**` note written by `audit finding --note`.
The seven improvised handoffs in 2026-06-13-codebase-quality-architecture now read
`tracked by 6fbwhsw024nr` instead of `deferred → tracked in task <slug>`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.