fix(list): let a named project lift the closed-project fold - #260
Merged
Merged
Conversation
Since #252 the --include-completed variants of today and anytime fold a task closed inside a closed project into the project's row, as logbook and trash do. The fold was unconditional, so it applied even when that project had been named: `things today --project "Launch v2" --include-completed` on a finished project returned nothing at all, because every closed child was folded and a project row never matches on the parent join. The fold exists so a closed project is one row rather than a row plus its contents. Naming the project is asking for the contents, which is the answer `things --project <uuid>` and `show --agent` already give. It now comes off in the two views as well. The filter reaches the WHERE composition through a whereOpts struct rather than string surgery: where() takes it, the status test reads it, and buildListQuery fills it at the one call site. With the parent pinned to the named project the lifted clause is a constant — true for an open project and false for a closed one — so dropping it can only affect the closed case, and naming an open project changes nothing. The golden file is the record of what moved: six of the two hundred view-and-filter combinations, all today and anytime at includeCompleted=true with the three filter shapes that carry a project, each differing only by the removed clause. Nothing else moved. The case does not occur in the data, so this is not measured against the app: no closed project has a child that is open and untrashed or closed today. Every unfiltered view returns exactly what it did before, checked against a build of origin/main. The rule comes from #229 and #235, where the fold was measured, and from the catch-all view, which has answered this way since #235. Two asymmetries stay, both recorded rather than guessed at. Naming a trashed project still returns nothing in these two views, because untrashedParent zeroes the result before the fold matters, and lifting that would change what the unfiltered views show. And --project takes a LIKE pattern, so a value of '%' lifts the fold for every project at once; no filter value is escaped anywhere, and the bare form already widens the same way. Closes #253
This was referenced Sep 10, 2026
ryanlewis
added a commit
that referenced
this pull request
Sep 10, 2026
…#264) Closes #243 ## What changed `printTasks`, `printProjects`, `printAreas` and `printTags` each declared a local row struct, measured every column with `lipgloss.Width` and padded with `padCol` before joining with a gap. Four copies of the same twenty lines, and the copy inside `printTasks` also carried the terminal-width column dropping. There is now one `table` in `internal/output/table.go`. Rows of cells go in, measured and padded lines come out. The column dropping is a `dropOrder` field naming the columns that may be given up, in the order they go, rather than a pair of booleans and a hand-written width sum. The rule the four copies shared is written down once: every column is padded to its widest cell except the last one declared, which goes out as it is. Dropping a column does not move that rule, which is why a narrow listing still pads its title column. That is the behaviour the copies had; it is now stated rather than implied. `printTasks` keeps its group-header logic, which no table can own, because it has to write a header between two rows. `table.lines()` hands back one line per row for exactly that, so the header fold from #205 is untouched. ## One place each for the kind word and the status text `t.Type == model.TypeProject` was branched on six times across `output.go` and `agent.go`. It is now `isProject(t)`, and the word that reaches the output is `kindWord(t)`, which takes "task" and "project" from model's type codec, the same source the JSON `type` field renders from. `kindWord` is deliberately not `TaskType.String()`. A heading, or a code Things has yet to write, reads as a task on these surfaces, which is what the CLI has always shown. The doc comment says so. `statusText` no longer keeps its own switch over the three statuses. It capitalises `model.Status.String()`, so the detail block, the JSON and the agent brief cannot drift apart, and the unrecognised code still renders "Unknown" because the codec already falls back to "unknown". `checklistLine` takes its "(cancelled)" from the same source. `statusIcon` stays as it is. `[ ]`, `[~]` and `[x]` are glyphs rather than the status words, and Markdown's two-state checkbox in a brief is a third thing again. Folding those together would have meant inventing a rendering, which this change is not for. ## Proof the output did not change The golden test went in first, in its own commit, before anything was refactored. `TestRenderGolden` renders a fixed fixture through every public entry point: `Print` for tasks, projects, areas and tags and for a `*model.Task`, `PrintTaskList` with and without a view label, `PrintTaskWithChecklist` for a to-do, a project and a repeating item, `PrintHint`, and `PrintAgentBrief` for a task, an open project, an empty project, a closed project, a repeating item and a note carrying its own code fence. Empty listings are cases too. Each case renders at both ends of the colour profile, so the ANSI is pinned as well as the text, and the task listings render at three terminal widths so both drop steps are covered. **The refactor commit does not touch `testdata/render_golden.txt`.** That is the claim, and the file's absence from that commit's diff is the evidence. The golden values were captured from unmodified rendering code. The one production change in that first commit is the seam the test needs: `termWidth` becomes a var, like `nowFn` beside it, so a test can pin a width instead of depending on how the test binary's stdout happens to be attached. The body is unchanged. `origin/main` moved to 61542c3 while this was in review, but #260 does not touch `internal/output`, so the captured bytes are still the bytes of the base this merges onto. Against the live database, 226 invocations of the built binary are byte-identical to `origin/main`, 1.8 MB of output in all: nine views plain, with `--include-completed`, as JSON, with `--color always` and with `--no-hints`; `projects`, `areas` and `tags` in the same three forms; and 43 real items through `show`, `show --agent`, `show --json` and `show --color always`. Exit codes are compared too. The database was read only through the binary. Every line in the golden file ends in `|`, because a padded column emits trailing spaces an editor or a hook would otherwise eat, and ANSI escapes are written `\e` so the file stays readable and greppable. ## Review `/code-review --fix` at high effort found one defect, and it was mine, in the golden test rather than in the refactor. `TestRenderGolden` returned early when the document matched and otherwise reported from inside its per-case loop. A difference the case splitter does not see, such as trailing whitespace or text before the first header, both of which it drops, left the loop with nothing to report and the test passed with `got != want`. Appending two newlines to the golden file reproduced it. For a test whose whole job is pinning bytes that is the one outcome it must not have, so it now fails loudly and names the regeneration command. The review separately re-derived the extraction rather than trusting the tests: that `table.fits` reproduces the old arithmetic including the two-step drop and the absent re-check after the last drop, that the last declared column is the one left unpadded whether or not it survived, that `kindWord` and `statusText` are equivalent to the switches they replace down to the unknown-code arm, and that the golden file is the same blob in both commits. It also confirmed the golden output is timezone-stable, checked under `TZ=Pacific/Kiritimati` and `TZ=UTC`, and that making `termWidth` a var introduces no race. ## Noted, not fixed `TestListQueryGoldenSQL` in `internal/db` has the same gap the review found here: it reports only from inside its per-case loop, so a difference outside a `### view=` block would pass. It is a separate package and a separate change, so it is left alone. ## Not touched No command surface moves, so `internal/skill/SKILL.md` and the pages under `docs/content/` have nothing to say about this. The `(project)` marker, the group header fold, the project icons and the agent brief's wording are all documented contracts and all unchanged.
ryanlewis
added a commit
that referenced
this pull request
Sep 10, 2026
Closes #262 ## What changed The list filters matched titles with `LIKE` and passed the value straight in as the pattern. `things --project '%'` matched every project — and since #260, lifted the closed-project fold for all of them at once — and a title holding a `%` or a `_` matched more than itself. Filter values are now escaped for `%`, `_` and the escape character, and every `LIKE` that takes one declares `ESCAPE`. Matching stays case-insensitive, which is what a name filter wants and what it did before; only the wildcards lose their meaning. The uuid arm keeps the raw value, since it is an equality test rather than a pattern. `things projects --area` takes the same flag and is escaped the same way, so the two commands cannot disagree about what a name matches. ## The golden file bounds it **100 of the 200** combinations moved: exactly the five filter-carrying shapes across ten views and both `--include-completed` states. No combination without a filter moved, and every one of the 100 differs only by the added `ESCAPE` clause — checked mechanically, line by line, not by eye. ## Measured against a base build Built `origin/main` and compared on the live database: | command | base | this branch | | --- | --- | --- | | `--project '%'` | 540 rows | 0 | | `--area 'Personal Projects'` | 43 | 43 | | `anytime` | 108 | 108 | No project, area or tag title in that database contains a `%` or a `_`, so no real name changes behaviour. ## A worse instance of the same bug, left for its own issue `/code-review` found that `FindTasksByTitle` and `SearchTasks` wrap the raw value in `%…%` with no escaping. That is the same gap on a more dangerous surface, because `GetTask` falls back to `FindTasksByTitle` and auto-selects when exactly one row comes back — so `things complete '20_30 review'` can resolve to and complete a task titled `20:30 review`. There are 4 open tasks with an underscore in the title on this database. The search half is live and measurable today: `things search '50%'` returns **45** rows, while the number of tasks whose title or notes contain the literal text `50%` is **0**. It is matching `%50%%`. Not fixed here. Escaping those changes what `show`, `complete` and `search` match, on a different command surface from the filters, and needs its own tests and a docs note — the README documents `<task>` as taking a title substring. Worth an issue. ## Docs None needed. The README already describes these flags as matching a name or UUID, which is what they now actually do; the pattern behaviour was accidental. The `<task>` substring claim is about the lookup path, which this does not touch. `make test` and `make lint` are clean.
ryanlewis
added a commit
that referenced
this pull request
Sep 10, 2026
#273) Since #260, naming a closed project with `--project` lifts the fold on `today` and `anytime`. Naming a trashed project still returned nothing there: the untrashed-parent guard zeroed the rows before the fold mattered, while the bare `things --project <uuid>` catch-all returned the project's contents. One parent, two answers. ## The rule A trashed or closed project's children are listed nowhere by default, as in the app, and an explicit `--project` is how they are reached. So the guard now comes off wherever `--project` names a project, not just in the catch-all. With the parent pinned to the named project the clause is a constant, true for an untrashed project and false for a trashed one, so lifting it can only ever affect the trashed case. Unfiltered views are untouched. `trash` is the exception and keeps the guard. Its rows are the ones thrown away on their own account, and a to-do thrown away out of a project that was later trashed is folded into that project's `trash` row and reachable nowhere, which the README, the docs and the agent skill all state. It also agrees with the catch-all's answer for the same project, which pins untrashed rows. `logbook` gains a trashed project's closed children, but not a *closed* project's: that view carries its own closed-parent fold, which only `today` and `anytime` lift, and this change does not touch it. ## Measurement first Taken on 10 Sep 2026 against my own data, read-only both ways. Asked directly, the app returns a trashed project's untrashed children whatever their status: one trashed project holding 2 untrashed open children answered 2, and one holding 12 children answered its 11 untrashed ones, leaving out the twelfth, which is in the Trash on its own account. That is the same rule the closed-parent case already encodes, so the measurement supports reaching the children by naming the project rather than narrowing the catch-all as the issue guessed. The database holds 3 open rows under trashed projects, spread over 2 trashed parents, all in the Anytime bucket with no start date. So `anytime --project <uuid>` is the view that gains them, and `today --project <uuid>` still returns none of them, correctly. ## Golden SQL 48 of the 200 cases move: the eight views that are neither the catch-all nor `trash`, each in its two `--include-completed` forms and its three project-bearing filter shapes. The only edit in every one of them is the removal of the trashed-parent clause. No case was added or removed, and the six `trash` project cases keep the clause. ## How tested `make test`, `make lint`, `make fmt`. New tests: the lift across `inbox`, `today`, `upcoming`, `anytime` and `deadlines`, each checking that the unfiltered view still hides the child, that naming the project by uuid and by title returns it, and that an untrashed project is unaffected; the same lift through a project heading; a trashed project's closed children reachable in `logbook`; and a trashed child of a trashed project staying out of both `trash --project` and the catch-all. Verified against my real database with the built binary, read-only: `anytime --project <trashed uuid>` returns 2 rows where it returned none, `logbook --project <trashed uuid>` returns 8 where it returned none, `trash --project <trashed uuid>` returns none, and the unfiltered totals are unchanged. Docs: the closed-project paragraphs in `commands.md`, `agents.md`, `SKILL.md` and the README now cover the trashed case too. Fixes #263
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.
Closes #253
What changed
Since #252 the
--include-completedvariants of today and anytime fold a task closed inside a closed project into the project's row, as logbook and trash do. The fold was unconditional, so it applied even when that project had been named:things today --project "Launch v2" --include-completedon a finished project returned nothing at all.The fold exists so a closed project is one row rather than a row plus its contents. Naming the project is asking for the contents — the answer
things --project <uuid>andshow --agentalready give. It now comes off in the two views as well.How it is plumbed
Through the spec seam #240 created rather than string surgery:
where()takes a two-fieldwhereOpts, the status test reads it, andbuildListQueryfills it at the one call site. With the parent pinned by the--projectpredicate the lifted clause is a constant — true for an open project, false for a closed one — so dropping it can only ever affect the closed case. Naming an open project changes nothing, which has its own test.The golden file is the record
Exactly 6 of the 200 view-and-filter combinations moved:
project,project+area+tag,allproject,project+area+tag,allEach differs only by the removed
AND NOT (COALESCE(p.status, 0) IN (2, 3)). No other view, filter shape or flag state moved.Not measured against the app, and why
The case does not occur in the data: no closed project has a child that is open and untrashed, or closed today. Every unfiltered view returns what it did before, which I checked by building
origin/mainand comparing rather than assuming. The rule comes from #229 and #235, where the fold itself was measured against the app, and from the catch-all view, which has answered this way since #235.One thing that looked like a regression was not:
anytime --include-completedreads 115 where my earlier notes said 114. The base build gives 115 too — a task was closed during the session — and it still matches the app exactly, membership and order.Two asymmetries left in, recorded not guessed
untrashedParentzeroes the result before the fold matters. The catch-all returns its contents. Fixing it would change what the unfiltered views show — 3 open rows in this data — which is beyond a change scoped to the--include-completedfold. Worth its own issue.--projecttakes a LIKE pattern, so--project '%'lifts the fold for every project at once. Pre-existing and not specific to this change: no filter value is escaped anywhere, and the barethings --project '%'already widens the same way.Docs
internal/skill/SKILL.md,docs/content/commands.mdanddocs/content/agents.mdeach said such a task is "in none of the three", which this makes false when the project is named. All three now say the fold holds for the unfiltered sweeps and comes off when the project is named, with the concrete command. The skill is scoped to closed projects specifically, since the trashed case still only works in the bare form.make testandmake lintare clean.