yuris/noctalia-timew

Fork 0

refactor(panel): add Timew command layer #3

Merged

yuris merged 2 commits from refactor/t3a-timew into main

2026-08-31 17:06:17 +00:00

yuris commented

2026-08-31 16:16:42 +00:00

Owner

refactor(panel): add a Timew command layer

Additive. Every timewarrior invocation the panel makes, and the parsing of every
line it prints, now has one home: a Timew table inserted immediately before
pokeBar(). Its callbacks hand back parsed Luau values; no caller has to see
stdout. Nothing calls it yet — the eight runAsync sites are untouched — so the
fetch functions keep their own copies of the same commands. That duplication is
deliberate and temporary, and the follow-up (T3b) deletes it by rewiring the
callers onto this table.

The point of doing it this way round: the commands are hard-won (the awk column
width trick that keeps deep work from becoming deep, the last-non-blank-field
total, the DOM walk's break condition) so they were copied verbatim rather than
rewritten, and the copy is proved byte-identical before anything moves.

Three layers, in dependency order.

Timew.cmd.* builds command strings and does nothing else. Pure, so the
awk programs can be asserted with no shell and no timew. Every command string
in the table comes from here.
Timew.parse* turns stdout into Luau values. Pure.
Timew.<name>(…, cb) is the only impure layer: a runAsync, a timeout,
and a parse.

Coverage: all eight command sites

Line numbers are on 38e8916 (this branch, rebased onto e3d69aa) and on
e3d69aa (main at the time of writing). Both moved when PR #2 inserted the
pure core, so neither matches the numbers this description carried at review.

`Timew` function	builder	replaces	line here	builder line	line on `main`
`Timew.activeInterval(cb)`	`cmd.activeInterval()`	`fetchActive`	558	473	392
`Timew.todayTotal(cb)`	`cmd.todayTotal()`	`fetchTotal`	565	480	410
`Timew.tags(cb)`	`cmd.tags()`	`fetchTags`	572	487	420
`Timew.dayTotals(days, tag, cb)`	`cmd.dayTotals(days, tag)`	`fetchDays`	579	500	439
`Timew.intervalsForDay(date, walkMax, cb)`	`cmd.intervalsForDay(date, walkMax)`	`loadDay`	586	515	466
`Timew.allIntervals(walkMax, tag, cb)`	`cmd.allIntervals(walkMax)`	`fetchAll`, the walk	595	532	510
`Timew.rangeTotal(tag, cb)`	`cmd.rangeTotal(tag)`	`fetchAll`, the all-time total	602	544	530
`Timew.exec(cmd, cb)`	`cmd.exec(cmd)`	`run`	620	549	551

Timew.tagArg(tag) (467) is the ninth pure function and the one that matters
most: it is the only place a user string reaches the shell, factored out of the
two copies of the same conditional that dayTotals and rangeTotal each
carried.

Parsers are separate from the commands that feed them, so each one can be checked
against a canned string with no timew and no shell: Timew.parseActive,
parseTotal, parseTags, parseDayTotals, parseIntervals, parseRangeTotal,
tagMatches. They call the pure core PR #2 added — Parse.trim,
Parse.unquote, Parse.clock, Parse.interval, Fmt.span — rather than
carrying a third copy of it.

shQuote is unchanged and still mandatory: runAsync takes a /bin/sh -c
string at this plugin API, never an argv array, so every interpolated tag or date
goes through it. Noted in a comment above the table, since the next reader's
first instinct will be to reach for an argv array that does not exist.

Verified

$ nix shell nixpkgs#luau -c luau test.luau
ok
exit=0

$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings
exit=0

$ git diff --numstat origin/main...HEAD
129     0       test.luau
291     0       timew/panel.luau

luau --check is not available in the nixpkgs build (Unrecognized option '--check'), so parse coverage comes from test.luau, which requires both
entries: a syntax error in bar.luau or panel.luau fails it.

test.luau gained 43 asserts across 129 added lines. Twenty cover the
parsers on canned stdout — quoted multi-word tags, untagged intervals, the idle
case, empty day totals, blank lines in the tag list, unparseable lines dropped,
the tag filter, and timew's No filtered data prose as zero. The rest are new
in the fixup: the eight frozen command literals, the bound defaults, and the
tagArg quoting asserts.

Parity, and this time it is reproducible

Committed asserts freeze all eight command strings, so the commands no longer
depend on a harness nobody can re-run. The harness still exists to prove those
literals are main's, and it is quoted below rather than described.

Both sides get the return block cut off and a tail concatenated on, so the
chunk's locals are in scope, with runAsync stubbed to record the command and
the timeout. Twenty-one invocations: the eight sites, the bound defaults, the
tag-carrying ones with and without a filter, and each of those again with a
hostile tag.

$ diff out_old.txt out_new.txt && echo "PARITY OK"
PARITY OK

$ md5sum out_old.txt out_new.txt
ce815eda9a44377c25248a29c5dcb2fe  out_old.txt
ce815eda9a44377c25248a29c5dcb2fe  out_new.txt

Identical byte for byte, including the awk programs, seq 0 13, the
case/break, 2>/dev/null placement, and all eight timeouts. The hostile tag
every quoted site was re-run with, and which produced no canary:

it's $(touch /tmp/t3adiff/PWNED) `touch /tmp/t3adiff/PWNED2` "q" ; touch /tmp/t3adiff/PWNED3 ; # \
 rm -rf /

It comes back out of tagArg as one single-quoted argument, apostrophe closed
and reopened the POSIX way, and ls PWNED PWNED2 PWNED3 finds nothing.

A second harness feeds one canned stdout to each old callback and to the matching
parser and compares the module state the callback wrote against the parser's
return (active/startEpoch/tags, total, recent, days, dayRows,
allRows, allTotal), plus exec against run on exit codes 0, 1 and 127:

85 comparisons, 0 differences
STATE EQUIVALENT (tags cap noted above, by design)

The one difference is tags, and it is deliberate — see below.

Decisions

Timew.exec(cmd, cb) takes a command string, not Timew.start(tag) /
Timew.stop(), and T3b should not add the pair either. bar.luau:215 builds
the identical two strings with its own shQuote (99) and cannot reach
panel.Timew, because the plugin API forbids require between entries. A
typed pair would put the tag's quoting inside the table for the panel and leave
it at the call site for the bar: two idioms for one operation. Changing that is
a decision about the no-require rule, not about this table. Written into the
comment above Timew.exec so it is not re-litigated.
Bounds default to their module constants; tags stay arguments.
days or DAYS, walkMax or DAY_WALK_MAX, walkMax or ALL_WALK_MAX. These
functions call runAsync, so they are impure by construction and the purity
argument never reached them, and the bounds were the file's only unquoted
interpolations. tag stays a parameter: filter genuinely varies at runtime.
Timew.tags returns every tag; RECENT_MAX capping stays with the caller.
How many fit on screen is not a parsing question. This is the one behavioural
difference from fetchTags, which capped inside its loop, and it is now
asserted rather than merely claimed: parseTags on six tags must return six.
A T3b rewire that forgets the cap fails the suite instead of overflowing a chip
row in a panel that cannot clip.
matchesFilter loses; Timew.tagMatches survives. matchesFilter (318)
reads the module's filter directly, which a parser taking its input as an
argument cannot do. Its three call sites (813, 1110, 1146) become
Timew.tagMatches(row.tag, filter) in T3b, and matchesFilter is deleted
there — not here, because wave 1 only adds, and two of the three sites are not
otherwise touched by the rewire. Recorded in the comment on tagMatches.
allIntervals filters the tag in Luau, not in timew. That is the existing
behaviour and the DOM walk takes no filter, but it means walkMax applies
before the filter: a tag whose occurrences are older than 200 intervals is
invisible in the list while rangeTotal still counts it. Preserved as-is,
documented at the function, and worth its own issue. If it is ever fixed the
fix is in timew, not in Luau.
activeInterval mirrors the epoch-conversion command actually in the file
(timew get dom.active.start, then date -d), not a
dom.active.start dom.active.duration dom.active.tags triple. date(1) does
the conversion so no Luau code parses a timestamp.
Parsers live flat on the same table (Timew.parseActive) rather than nested
under Timew.parse.*. One table, one lookup, no ceremony. Timew.cmd is
nested because it is a different kind of thing: strings, not behaviour.

Why the command strings are frozen literals

Frozen literals are change-detector tests, which are usually not worth having.
These earn it. The awk programs are hand-tuned, no argument varies them, and
after T3b deletes fetchActive…run there is nothing left in the repo to diff
them against. A typo in any of them passes every other check.

They are reachable as values only because Timew.cmd is pure. A harness cannot
be committed instead: the luau CLI gives each required module its own global
table, so a runAsync stubbed from test.luau is invisible inside
panel.luau. That is why the strings had to become values rather than the
consumer becoming stubbable.

The tagArg asserts are not change detectors. They are the only automated check
that the injection path stays closed, and they cover an apostrophe, a newline, a
command substitution and a '; rm -rf /; # break-out.

Each new assert was checked for non-vacuity by mutation: NR==3→NR==4,
$NF→$1, DAY_WALK_MAX→61, dropping the empty-string arm of tagArg, and
re-adding the RECENT_MAX cap to parseTags each fail the suite, at the
expected assert.

# refactor(panel): add a Timew command layer Additive. Every timewarrior invocation the panel makes, and the parsing of every line it prints, now has one home: a `Timew` table inserted immediately before `pokeBar()`. Its callbacks hand back parsed Luau values; no caller has to see stdout. Nothing calls it yet — the eight `runAsync` sites are untouched — so the fetch functions keep their own copies of the same commands. That duplication is deliberate and temporary, and the follow-up (T3b) deletes it by rewiring the callers onto this table. The point of doing it this way round: the commands are hard-won (the awk column width trick that keeps `deep work` from becoming `deep`, the last-non-blank-field total, the DOM walk's break condition) so they were copied verbatim rather than rewritten, and the copy is proved byte-identical before anything moves. Three layers, in dependency order. - **`Timew.cmd.*`** builds command strings and does nothing else. Pure, so the awk programs can be asserted with no shell and no timew. Every command string in the table comes from here. - **`Timew.parse*`** turns stdout into Luau values. Pure. - **`Timew.<name>(…, cb)`** is the only impure layer: a `runAsync`, a timeout, and a parse. ## Coverage: all eight command sites Line numbers are on `38e8916` (this branch, rebased onto `e3d69aa`) and on `e3d69aa` (`main` at the time of writing). Both moved when PR #2 inserted the pure core, so neither matches the numbers this description carried at review. | `Timew` function | builder | replaces | line here | builder line | line on `main` | | --- | --- | --- | --- | --- | --- | | `Timew.activeInterval(cb)` | `cmd.activeInterval()` | `fetchActive` | 558 | 473 | 392 | | `Timew.todayTotal(cb)` | `cmd.todayTotal()` | `fetchTotal` | 565 | 480 | 410 | | `Timew.tags(cb)` | `cmd.tags()` | `fetchTags` | 572 | 487 | 420 | | `Timew.dayTotals(days, tag, cb)` | `cmd.dayTotals(days, tag)` | `fetchDays` | 579 | 500 | 439 | | `Timew.intervalsForDay(date, walkMax, cb)` | `cmd.intervalsForDay(date, walkMax)` | `loadDay` | 586 | 515 | 466 | | `Timew.allIntervals(walkMax, tag, cb)` | `cmd.allIntervals(walkMax)` | `fetchAll`, the walk | 595 | 532 | 510 | | `Timew.rangeTotal(tag, cb)` | `cmd.rangeTotal(tag)` | `fetchAll`, the all-time total | 602 | 544 | 530 | | `Timew.exec(cmd, cb)` | `cmd.exec(cmd)` | `run` | 620 | 549 | 551 | `Timew.tagArg(tag)` (467) is the ninth pure function and the one that matters most: it is the only place a user string reaches the shell, factored out of the two copies of the same conditional that `dayTotals` and `rangeTotal` each carried. Parsers are separate from the commands that feed them, so each one can be checked against a canned string with no timew and no shell: `Timew.parseActive`, `parseTotal`, `parseTags`, `parseDayTotals`, `parseIntervals`, `parseRangeTotal`, `tagMatches`. They call the pure core PR #2 added — `Parse.trim`, `Parse.unquote`, `Parse.clock`, `Parse.interval`, `Fmt.span` — rather than carrying a third copy of it. `shQuote` is unchanged and still mandatory: `runAsync` takes a `/bin/sh -c` string at this plugin API, never an argv array, so every interpolated tag or date goes through it. Noted in a comment above the table, since the next reader's first instinct will be to reach for an argv array that does not exist. ## Verified ``` $ nix shell nixpkgs#luau -c luau test.luau ok exit=0 $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings exit=0 $ git diff --numstat origin/main...HEAD 129 0 test.luau 291 0 timew/panel.luau ``` `luau --check` is not available in the nixpkgs build (`Unrecognized option '--check'`), so parse coverage comes from `test.luau`, which requires both entries: a syntax error in `bar.luau` or `panel.luau` fails it. `test.luau` gained **43 asserts across 129 added lines**. Twenty cover the parsers on canned stdout — quoted multi-word tags, untagged intervals, the idle case, empty day totals, blank lines in the tag list, unparseable lines dropped, the tag filter, and timew's `No filtered data` prose as zero. The rest are new in the fixup: the eight frozen command literals, the bound defaults, and the `tagArg` quoting asserts. ### Parity, and this time it is reproducible Committed asserts freeze all eight command strings, so the commands no longer depend on a harness nobody can re-run. The harness still exists to prove those literals are `main`'s, and it is quoted below rather than described. Both sides get the `return` block cut off and a tail concatenated on, so the chunk's locals are in scope, with `runAsync` stubbed to record the command and the timeout. Twenty-one invocations: the eight sites, the bound defaults, the tag-carrying ones with and without a filter, and each of those again with a hostile tag. ``` $ diff out_old.txt out_new.txt && echo "PARITY OK" PARITY OK $ md5sum out_old.txt out_new.txt ce815eda9a44377c25248a29c5dcb2fe out_old.txt ce815eda9a44377c25248a29c5dcb2fe out_new.txt ``` Identical byte for byte, including the awk programs, `seq 0 13`, the `case`/`break`, `2>/dev/null` placement, and all eight timeouts. The hostile tag every quoted site was re-run with, and which produced no canary: ``` it's $(touch /tmp/t3adiff/PWNED) `touch /tmp/t3adiff/PWNED2` "q" ; touch /tmp/t3adiff/PWNED3 ; # \ rm -rf / ``` It comes back out of `tagArg` as one single-quoted argument, apostrophe closed and reopened the POSIX way, and `ls PWNED PWNED2 PWNED3` finds nothing. A second harness feeds one canned stdout to each old callback and to the matching parser and compares the module state the callback wrote against the parser's return (`active`/`startEpoch`/`tags`, `total`, `recent`, `days`, `dayRows`, `allRows`, `allTotal`), plus `exec` against `run` on exit codes 0, 1 and 127: ``` 85 comparisons, 0 differences STATE EQUIVALENT (tags cap noted above, by design) ``` The one difference is `tags`, and it is deliberate — see below. ## Decisions - **`Timew.exec(cmd, cb)` takes a command string, not `Timew.start(tag)` / `Timew.stop()`, and T3b should not add the pair either.** `bar.luau:215` builds the identical two strings with its own `shQuote` (99) and cannot reach `panel.Timew`, because the plugin API forbids `require` between entries. A typed pair would put the tag's quoting inside the table for the panel and leave it at the call site for the bar: two idioms for one operation. Changing that is a decision about the no-require rule, not about this table. Written into the comment above `Timew.exec` so it is not re-litigated. - **Bounds default to their module constants; tags stay arguments.** `days or DAYS`, `walkMax or DAY_WALK_MAX`, `walkMax or ALL_WALK_MAX`. These functions call `runAsync`, so they are impure by construction and the purity argument never reached them, and the bounds were the file's only unquoted interpolations. `tag` stays a parameter: `filter` genuinely varies at runtime. - **`Timew.tags` returns every tag; `RECENT_MAX` capping stays with the caller.** How many fit on screen is not a parsing question. This is the one behavioural difference from `fetchTags`, which capped inside its loop, and it is now asserted rather than merely claimed: `parseTags` on six tags must return six. A T3b rewire that forgets the cap fails the suite instead of overflowing a chip row in a panel that cannot clip. - **`matchesFilter` loses; `Timew.tagMatches` survives.** `matchesFilter` (318) reads the module's `filter` directly, which a parser taking its input as an argument cannot do. Its three call sites (813, 1110, 1146) become `Timew.tagMatches(row.tag, filter)` in T3b, and `matchesFilter` is deleted there — not here, because wave 1 only adds, and two of the three sites are not otherwise touched by the rewire. Recorded in the comment on `tagMatches`. - **`allIntervals` filters the tag in Luau, not in timew.** That is the existing behaviour and the DOM walk takes no filter, but it means `walkMax` applies before the filter: a tag whose occurrences are older than 200 intervals is invisible in the list while `rangeTotal` still counts it. Preserved as-is, documented at the function, and worth its own issue. If it is ever fixed the fix is in timew, not in Luau. - **`activeInterval` mirrors the epoch-conversion command actually in the file** (`timew get dom.active.start`, then `date -d`), not a `dom.active.start dom.active.duration dom.active.tags` triple. `date(1)` does the conversion so no Luau code parses a timestamp. - **Parsers live flat on the same table** (`Timew.parseActive`) rather than nested under `Timew.parse.*`. One table, one lookup, no ceremony. `Timew.cmd` is nested because it is a different kind of thing: strings, not behaviour. ### Why the command strings are frozen literals Frozen literals are change-detector tests, which are usually not worth having. These earn it. The awk programs are hand-tuned, no argument varies them, and after T3b deletes `fetchActive`…`run` there is nothing left in the repo to diff them against. A typo in any of them passes every other check. They are reachable as values only because `Timew.cmd` is pure. A harness cannot be committed instead: the luau CLI gives each required module its own global table, so a `runAsync` stubbed from `test.luau` is invisible inside `panel.luau`. That is why the strings had to become values rather than the consumer becoming stubbable. The `tagArg` asserts are not change detectors. They are the only automated check that the injection path stays closed, and they cover an apostrophe, a newline, a command substitution and a `'; rm -rf /; #` break-out. Each new assert was checked for non-vacuity by mutation: `NR==3`→`NR==4`, `$NF`→`$1`, `DAY_WALK_MAX`→`61`, dropping the empty-string arm of `tagArg`, and re-adding the `RECENT_MAX` cap to `parseTags` each fail the suite, at the expected assert.

yuris added 1 commit

2026-08-31 16:16:42 +00:00

refactor(panel): add Timew command layer 175d69f1a2

Every timewarrior invocation and every line of its output now has one home:
a Timew table whose callbacks hand back parsed Luau values instead of raw
stdout. Command strings, awk tricks, DOM-walk bounds and timeouts are copied
from the fetch functions unchanged.

Nothing calls it yet, so the fetch functions keep their own copies for now;
rewiring them onto Timew is the next change.

yuris reviewed

2026-08-31 16:32:45 +00:00

yuris left a comment

Verdict: REQUEST_CHANGES. (Posted as a comment review: Forgejo refuses a
REQUEST_CHANGES state on one's own pull request, so the verdict is stated here
rather than carried as review state.)

The copy is good. I rebuilt the parity harness you described and threw away, and
it agrees with you: all eight command strings and their timeouts are byte-identical
to the ones on main, and so is every parser output except one. shQuote holds
against everything I could throw at it, including a newline-embedded rm -rf and
three $(touch) canaries, none of which fired. The diff is purely additive (zero
deleted lines), touches only timew/panel.luau and test.luau, and no excluded
file is committed. Nothing here is a runtime defect, and I could not make one
happen. I am requesting changes on three narrow points, all of them cheaper now
than after T3b: the command strings this PR exists to preserve have no check in
the repo and cannot get one without a small change to the table's shape, the one
behaviour delta you did introduce has no test guarding it, and three parameters
exist to serve callers that never will.

Findings

1. The parity evidence cannot be committed as written, and after T3b nothing can re-derive it — `timew/panel.luau:279`, `:351`

I rebuilt your harness and it agrees with you, so this is not doubt about the
result. It is that the result lives only in the PR description. Once T3b deletes
fetchActive…run, there is nothing left to diff the copy against, and the
strings that make this PR worth doing lose their last check.

They have none today. Your 20 asserts cover the parsers; every trick the
description brags about lives in the command strings: the
NR==3{w=length($1)} column cut that keeps deep work whole, the NF{l=$NF}
last-non-blank-field, the case/break early exit, the 14-iteration loop, the
2>/dev/null placement, and the shQuote composition. A typo in any of them
passes luau test.luau and passes noctalia plugins lint timew today.

Before you reach for the obvious fix: you cannot move the harness into
test.luau by stubbing runAsync, because the luau CLI gives every required
module its own global table. A stub assigned in the test is invisible inside
panel.luau:

$ cat env/mod.luau
return { peek = function() return tostring(INJECTED) end }

$ cat env_probe.luau
INJECTED = "from-main"
local m = require("./env/mod")
print("module sees INJECTED = " .. m.peek())
print("main sees INJECTED   = " .. tostring(INJECTED))

$ luau env_probe.luau
module sees INJECTED = nil
main sees INJECTED   = from-main

That is precisely why your harness had to sed the return block off and
concatenate: one chunk, one global table. It works, and it is also why it can
never be a committed test.

The way out is to stop stubbing the consumer and make the strings pure values.
Two changes, both of which shorten the file:

-- The only place a user string reaches the shell. Pure and exported, because a
-- required module gets its own globals: test.luau cannot stub runAsync to catch
-- a built command, so the quoting has to be reachable without one.
function Timew.tagArg(tag)
	return if tag == nil or tag == "" then "" else " " .. shQuote(tag)
end

Timew.cmd = {}

function Timew.cmd.rangeTotal(tag)
	return "timew summary :all" .. Timew.tagArg(tag) .. " 2>/dev/null | awk 'NF{l=$NF} END{print l}'"
end

function Timew.rangeTotal(tag, cb)
	noctalia.runAsync(Timew.cmd.rangeTotal(tag), function(result)
		cb(Timew.parseRangeTotal(result.stdout))
	end, 10000)
end

tagArg also deletes the duplicated conditional now sitting at 299 and 352.

I prototyped this on a copy — tagArg plus cmd.todayTotal and cmd.rangeTotal
wired through — and re-ran both harnesses against it:

$ diff out_old.txt out_proto.txt && echo "PROTOTYPE COMMANDS == MAIN COMMANDS"
PROTOTYPE COMMANDS == MAIN COMMANDS

$ diff st_new.txt st_proto.txt && echo "PROTOTYPE STATE == PR STATE"
PROTOTYPE STATE == PR STATE

$ luau probe3.luau
pure-builder probe ok

The asserts that probe runs, which is what belongs in test.luau:

assert(T.cmd.todayTotal() == "timew summary :day 2>/dev/null | awk 'NF{l=$NF} END{print l}'")
assert(T.cmd.rangeTotal("deep work") == "timew summary :all 'deep work' 2>/dev/null | awk 'NF{l=$NF} END{print l}'")
assert(T.tagArg(nil) == "" and T.tagArg("") == "", "no tag adds no argument")
assert(T.tagArg("it's") == " 'it'\\''s'", "an apostrophe cannot close the quote")
assert(T.tagArg("$(id) `id`") == " '$(id) `id`'", "substitution is inert inside single quotes")

Extend Timew.cmd to all eight and freeze each literal. Frozen literals are
change-detector tests, which I would normally argue against, but a hand-tuned awk
program that no argument can vary is one of the few places they earn it — and the
tagArg asserts are not change-detectors at all, they are the only automated
check that the injection path stays closed.

If you would rather keep this PR a pure move, say so and I will sign it off with
Timew.cmd as T3b's first commit instead. What I do not want is for it to land
after the old strings are gone.

2. `parseTags` drops the `RECENT_MAX` cap, and nothing tests it — `timew/panel.luau:222` vs `:462`

This is the only place where old and new disagree. Feeding both the same six-tag
stdout:

old  tags  {1=deep work,2=focus,3=admin,4=planning}
new  tags  {1=deep work,2=focus,3=admin,4=planning,5=review}

fetchTags caps inside the loop (#recent < RECENT_MAX, line 462); parseTags
returns everything. Your description calls this deliberate and I agree with the
call — how many fit on screen is not a parsing question. The problem is that it is
deliberate only in prose. When T3b rewires fetchTags onto Timew.tags and
forgets the cap, recent gets every tag the user has ever used, the chip row
overflows a panel that cannot clip, and the whole suite stays green.

The assert that stops it, next to the existing parseTags one:

local many = T.parseTags("a\nb\nc\nd\ne\nf\n")
assert(#many == 6, "parseTags does not cap; RECENT_MAX is the caller's job")

Cheap, and it turns a paragraph in a PR description into something T3b trips over.

3. `days` and `walkMax` are parameters with exactly one possible argument — `timew/panel.luau:298`, `:316`, `:337`

Answering your 5c below, but it is also the one bit of over-engineering here.
dayTotals(days, …) will only ever be handed DAYS, intervalsForDay(…, walkMax, …)
only DAY_WALK_MAX, allIntervals(walkMax, …) only ALL_WALK_MAX. All three
constants are module-level in this same file, forty lines up.

The purity argument does not reach them: these three functions call
noctalia.runAsync, so they are impure by construction, and the genuinely pure
part (parse*) already takes no bounds and needs none. What the parameter does buy
is the file's only unquoted interpolation sites — .. walkMax .. and
.. (days - 1) .. go into the command string raw. Harmless today because both are
numeric constants, and identical to main, so this is not a regression. But it is
a quoting exception that exists to serve a caller that will never exist.

Default them and the call sites get shorter without losing anything:

function Timew.allIntervals(walkMax, tag, cb)
	walkMax = walkMax or ALL_WALK_MAX

Keep the tag parameters exactly as they are. filter genuinely changes at
runtime, and that is a real argument.

4. Every line number in the coverage table points at the wrong code

The "line in this PR" column is the old line plus 203 in every row, so it lands on
the function being replaced, not the new one. Line 428 is fetchActive, not
Timew.activeInterval. T3b will navigate by this table.

Actual lines: activeInterval 269, todayTotal 279, tags 288, dayTotals 298,
intervalsForDay 316, allIntervals 337, rangeTotal 351, exec 363. The
line on main column is correct throughout. While you are in there: the
description says 30 asserts, the file has 20 across 35 added lines.

5. Give `matchesFilter` a disposition before it becomes permanent — `timew/panel.luau:148` and `:192`

tagMatches earns its place: it is the parameterised form, and a pure parser
cannot read module state. But matchesFilter has three call sites (556, 853, 889),
and only 556 dies when T3b rewires fetchAll. Left alone, the file keeps two
spellings of one predicate forever.

Say in the T3b description that 853 and 889 become
Timew.tagMatches(row.tag, filter) and matchesFilter is deleted. Not a change
for this PR — wave 1 is additive — but a decision worth writing down while the
reason is fresh.

Verified

Both commands, in .worktrees/t3a-timew, reproduce what you reported:

$ nix shell nixpkgs#luau -c luau test.luau
ok
exit=0

$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings
exit=0

Command parity. I rebuilt your harness (sed the return block off each
side, append a tail so the chunk locals are in scope, stub runAsync to record
cmd and timeout) and diffed the two sides. Fifteen invocations: the eight
sites, the three tag-carrying ones repeated with a filter, and three repeated with
a hostile tag.

$ diff out_old.txt out_new.txt && echo "PARITY OK"
PARITY OK

$ md5sum out_old.txt out_new.txt
7ed05d7dd01de8271c29d5f17b44b94e  out_old.txt
7ed05d7dd01de8271c29d5f17b44b94e  out_new.txt

Identical byte for byte, including the awk programs, seq 0 13, the
case/break, 2>/dev/null placement, and all eight timeouts.

Quoting, the safety ruling: safe. shQuote is textbook POSIX single-quoting
(' becomes '\''), which is total over every byte except NUL. I tried to break
out of it and could not. The tag shown below went through rangeTotal and
dayTotals against a stub timew on PATH that prints its argv:

$ ls PWNED PWNED2 PWNED3
ls: cannot access 'PWNED': No such file or directory
ls: cannot access 'PWNED2': No such file or directory
ls: cannot access 'PWNED3': No such file or directory

$ cat argv.txt
### rangeTotal
ARGC=3
ARG[summary]
ARG[:all]
ARG[it's $(touch /tmp/parity/PWNED) `touch /tmp/parity/PWNED2` "q" ; touch /tmp/parity/PWNED3 ; #]

One argv element, byte-identical to the input, no canary created. A separate
round-trip through /bin/sh with a tag carrying an embedded newline, a literal
rm -rf line, a backtick pair and a trailing backslash came back byte for byte
unchanged, and left the canary file it named alive.

Every user-reachable interpolation is quoted: the tag in dayTotals (299),
rangeTotal (352), and the date in intervalsForDay (318). allIntervals never
puts the tag in the command at all. The only raw interpolations are the numeric
bounds in finding 3, which no caller controls.

Parser parity. Same harness, feeding one canned stdout to each old callback
and to the matching parser, then comparing the module state against the return:

active         {active=true,startEpoch=1756662533,tags=deep work}
idle           {active=nil,startEpoch=0,tags=}
total          0:12:34
total-empty    0:00:00
tags           {…}   <-- differs, finding 2
days           {1={date=2026-08-31,dow=Mon,seconds=754}}
dayRows        {1={at=17:48,…,tag=focus},2={…,tag=deep work},3={…,tag=(untagged)}}
allRows        {1={at=09:00,date=30/08,duration=2:00,tag=deep work}}
allTotal       5:20
allTotal-empty 0:00

Identical on every row but tags. The parsers are genuinely pure: they ran in a
harness that never touched module state and never called render.

Structure. git diff --numstat main refactor/t3a-timew reports 35 0 test.luau
and 204 0 timew/panel.luau: 239 insertions, zero deletions, so no live comment was
removed. Only those two files are modified; git ls-tree -r on the branch shows no
*.md, no secretspec.toml, no forge.sh. The table sits at 185–370, immediately
before pokeBar() at 373.
Timew is added to the return table, which the host ignores, so no handler name,
settings key, or IPC event changed.

Quirk coverage, since you asked what the asserts miss. Blank lines,
(untagged), quoted multi-word tags, span-versus-prose, and empty output are all
covered. Three gaps. The timew tags header and rule-width trick lives in the awk
rather than the parser, so only finding 1 can reach it. The other two are parser
asserts you could add today: parseDayTotals is never fed timew's prose day
(2026-08-30 Sun No filtered data), which the ^(%S+)%s+(%S+)%s+(%S+)$ pattern
drops rather than zeroes — the shell case is the only thing saving it, so an
assert there documents why that case must never be deleted; and
parseRangeTotal is asserted with No filtered data where timew actually prints
No filtered data found in the range ….

Answers

5a. Timew.exec(cmd, cb) or Timew.start(tag) / Timew.stop()? Keep exec,
and do not add the typed pair in T3b either. Inside panel.luau the pair would
be clean — three call sites (599, 745, 1176), all of which would lose their
shQuote. The reason not to is bar.luau:142, which builds the identical
"timew start " .. shQuote(…) and "timew stop" with its own shQuote at line
34, and cannot reach panel.Timew because the plugin API forbids require
between entries. A typed pair would therefore exist for one of the two callers and
not the other: the tag's quoting moves inside the table for the panel and stays at
the call site for the bar, which is two idioms for one operation instead of one.
exec also mirrors bar's runAsync(cmd .. " 2>&1", …) exactly. If you ever want
the typed pair it has to be written into both entries, and that is a decision about
the no-require rule, not about this table.

5b. walkMax bounding the walk before the Luau tag filter: keep, and it is
already documented. The comment at 335–337 states the consequence precisely, and
the pairing with rangeTotal is the honest mitigation: a truncated list still
reports a true total, because the number comes from timew rather than from the
rows. This is main's behaviour, and preserving it is exactly the wave 1 contract.
Please make sure T3b does not quietly "improve" it, since a rewire that changes
what the list contains is drift wearing a refactor's clothes. Worth its own issue,
and if it is ever fixed the fix is in timew, not in Luau: give the DOM walk a
timew get filtered range so the bound applies after the tag, not before.

5c. Counts and bounds as parameters: right for tag, needless for the bounds.
See finding 3. Default days, walkMax to the module constants and keep the tag
arguments.

Also asked, on over-engineering. No Timew function is unused: all seven
parsers feed a command function or an assert, and all eight command functions map
to a known caller. tagMatches earns its place over matchesFilter for the reason
in finding 5. There is no dead nil-callback path — every function calls cb(…)
unconditionally, with no if cb then guard, which is the right call. The only
indirection I would cut is the bounds.

**Verdict: REQUEST_CHANGES.** (Posted as a comment review: Forgejo refuses a `REQUEST_CHANGES` state on one's own pull request, so the verdict is stated here rather than carried as review state.) The copy is good. I rebuilt the parity harness you described and threw away, and it agrees with you: all eight command strings and their timeouts are byte-identical to the ones on `main`, and so is every parser output except one. `shQuote` holds against everything I could throw at it, including a newline-embedded `rm -rf` and three `$(touch)` canaries, none of which fired. The diff is purely additive (zero deleted lines), touches only `timew/panel.luau` and `test.luau`, and no excluded file is committed. Nothing here is a runtime defect, and I could not make one happen. I am requesting changes on three narrow points, all of them cheaper now than after T3b: the command strings this PR exists to preserve have no check in the repo and cannot get one without a small change to the table's shape, the one behaviour delta you did introduce has no test guarding it, and three parameters exist to serve callers that never will. ## Findings ### 1. The parity evidence cannot be committed as written, and after T3b nothing can re-derive it — `timew/panel.luau:279`, `:351` I rebuilt your harness and it agrees with you, so this is not doubt about the result. It is that the result lives only in the PR description. Once T3b deletes `fetchActive`…`run`, there is nothing left to diff the copy against, and the strings that make this PR worth doing lose their last check. They have none today. Your 20 asserts cover the *parsers*; every trick the description brags about lives in the *command strings*: the `NR==3{w=length($1)}` column cut that keeps `deep work` whole, the `NF{l=$NF}` last-non-blank-field, the `case`/`break` early exit, the 14-iteration loop, the `2>/dev/null` placement, and the `shQuote` composition. A typo in any of them passes `luau test.luau` and passes `noctalia plugins lint timew` today. Before you reach for the obvious fix: you cannot move the harness into `test.luau` by stubbing `runAsync`, because the luau CLI gives every required module its own global table. A stub assigned in the test is invisible inside `panel.luau`: ``` $ cat env/mod.luau return { peek = function() return tostring(INJECTED) end } $ cat env_probe.luau INJECTED = "from-main" local m = require("./env/mod") print("module sees INJECTED = " .. m.peek()) print("main sees INJECTED = " .. tostring(INJECTED)) $ luau env_probe.luau module sees INJECTED = nil main sees INJECTED = from-main ``` That is precisely why your harness had to `sed` the `return` block off and concatenate: one chunk, one global table. It works, and it is also why it can never be a committed test. The way out is to stop stubbing the consumer and make the strings pure values. Two changes, both of which shorten the file: ```lua -- The only place a user string reaches the shell. Pure and exported, because a -- required module gets its own globals: test.luau cannot stub runAsync to catch -- a built command, so the quoting has to be reachable without one. function Timew.tagArg(tag) return if tag == nil or tag == "" then "" else " " .. shQuote(tag) end Timew.cmd = {} function Timew.cmd.rangeTotal(tag) return "timew summary :all" .. Timew.tagArg(tag) .. " 2>/dev/null | awk 'NF{l=$NF} END{print l}'" end function Timew.rangeTotal(tag, cb) noctalia.runAsync(Timew.cmd.rangeTotal(tag), function(result) cb(Timew.parseRangeTotal(result.stdout)) end, 10000) end ``` `tagArg` also deletes the duplicated conditional now sitting at 299 and 352. I prototyped this on a copy — `tagArg` plus `cmd.todayTotal` and `cmd.rangeTotal` wired through — and re-ran both harnesses against it: ``` $ diff out_old.txt out_proto.txt && echo "PROTOTYPE COMMANDS == MAIN COMMANDS" PROTOTYPE COMMANDS == MAIN COMMANDS $ diff st_new.txt st_proto.txt && echo "PROTOTYPE STATE == PR STATE" PROTOTYPE STATE == PR STATE $ luau probe3.luau pure-builder probe ok ``` The asserts that probe runs, which is what belongs in `test.luau`: ```lua assert(T.cmd.todayTotal() == "timew summary :day 2>/dev/null | awk 'NF{l=$NF} END{print l}'") assert(T.cmd.rangeTotal("deep work") == "timew summary :all 'deep work' 2>/dev/null | awk 'NF{l=$NF} END{print l}'") assert(T.tagArg(nil) == "" and T.tagArg("") == "", "no tag adds no argument") assert(T.tagArg("it's") == " 'it'\\''s'", "an apostrophe cannot close the quote") assert(T.tagArg("$(id) `id`") == " '$(id) `id`'", "substitution is inert inside single quotes") ``` Extend `Timew.cmd` to all eight and freeze each literal. Frozen literals are change-detector tests, which I would normally argue against, but a hand-tuned awk program that no argument can vary is one of the few places they earn it — and the `tagArg` asserts are not change-detectors at all, they are the only automated check that the injection path stays closed. If you would rather keep this PR a pure move, say so and I will sign it off with `Timew.cmd` as T3b's first commit instead. What I do not want is for it to land after the old strings are gone. ### 2. `parseTags` drops the `RECENT_MAX` cap, and nothing tests it — `timew/panel.luau:222` vs `:462` This is the only place where old and new disagree. Feeding both the same six-tag stdout: ``` old tags {1=deep work,2=focus,3=admin,4=planning} new tags {1=deep work,2=focus,3=admin,4=planning,5=review} ``` `fetchTags` caps inside the loop (`#recent < RECENT_MAX`, line 462); `parseTags` returns everything. Your description calls this deliberate and I agree with the call — how many fit on screen is not a parsing question. The problem is that it is deliberate only in prose. When T3b rewires `fetchTags` onto `Timew.tags` and forgets the cap, `recent` gets every tag the user has ever used, the chip row overflows a panel that cannot clip, and the whole suite stays green. The assert that stops it, next to the existing `parseTags` one: ```lua local many = T.parseTags("a\nb\nc\nd\ne\nf\n") assert(#many == 6, "parseTags does not cap; RECENT_MAX is the caller's job") ``` Cheap, and it turns a paragraph in a PR description into something T3b trips over. ### 3. `days` and `walkMax` are parameters with exactly one possible argument — `timew/panel.luau:298`, `:316`, `:337` Answering your 5c below, but it is also the one bit of over-engineering here. `dayTotals(days, …)` will only ever be handed `DAYS`, `intervalsForDay(…, walkMax, …)` only `DAY_WALK_MAX`, `allIntervals(walkMax, …)` only `ALL_WALK_MAX`. All three constants are module-level in this same file, forty lines up. The purity argument does not reach them: these three functions call `noctalia.runAsync`, so they are impure by construction, and the genuinely pure part (`parse*`) already takes no bounds and needs none. What the parameter does buy is the file's only unquoted interpolation sites — `.. walkMax ..` and `.. (days - 1) ..` go into the command string raw. Harmless today because both are numeric constants, and identical to `main`, so this is not a regression. But it is a quoting exception that exists to serve a caller that will never exist. Default them and the call sites get shorter without losing anything: ```lua function Timew.allIntervals(walkMax, tag, cb) walkMax = walkMax or ALL_WALK_MAX ``` Keep the `tag` parameters exactly as they are. `filter` genuinely changes at runtime, and that is a real argument. ### 4. Every line number in the coverage table points at the wrong code The "line in this PR" column is the old line plus 203 in every row, so it lands on the function being *replaced*, not the new one. Line 428 is `fetchActive`, not `Timew.activeInterval`. T3b will navigate by this table. Actual lines: `activeInterval` 269, `todayTotal` 279, `tags` 288, `dayTotals` 298, `intervalsForDay` 316, `allIntervals` 337, `rangeTotal` 351, `exec` 363. The `line on main` column is correct throughout. While you are in there: the description says 30 asserts, the file has 20 across 35 added lines. ### 5. Give `matchesFilter` a disposition before it becomes permanent — `timew/panel.luau:148` and `:192` `tagMatches` earns its place: it is the parameterised form, and a pure parser cannot read module state. But `matchesFilter` has three call sites (556, 853, 889), and only 556 dies when T3b rewires `fetchAll`. Left alone, the file keeps two spellings of one predicate forever. Say in the T3b description that 853 and 889 become `Timew.tagMatches(row.tag, filter)` and `matchesFilter` is deleted. Not a change for this PR — wave 1 is additive — but a decision worth writing down while the reason is fresh. ## Verified Both commands, in `.worktrees/t3a-timew`, reproduce what you reported: ``` $ nix shell nixpkgs#luau -c luau test.luau ok exit=0 $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings exit=0 ``` **Command parity.** I rebuilt your harness (`sed` the `return` block off each side, append a tail so the chunk locals are in scope, stub `runAsync` to record `cmd` and `timeout`) and diffed the two sides. Fifteen invocations: the eight sites, the three tag-carrying ones repeated with a filter, and three repeated with a hostile tag. ``` $ diff out_old.txt out_new.txt && echo "PARITY OK" PARITY OK $ md5sum out_old.txt out_new.txt 7ed05d7dd01de8271c29d5f17b44b94e out_old.txt 7ed05d7dd01de8271c29d5f17b44b94e out_new.txt ``` Identical byte for byte, including the awk programs, `seq 0 13`, the `case`/`break`, `2>/dev/null` placement, and all eight timeouts. **Quoting, the safety ruling: safe.** `shQuote` is textbook POSIX single-quoting (`'` becomes `'\''`), which is total over every byte except NUL. I tried to break out of it and could not. The tag shown below went through `rangeTotal` and `dayTotals` against a stub `timew` on `PATH` that prints its argv: ``` $ ls PWNED PWNED2 PWNED3 ls: cannot access 'PWNED': No such file or directory ls: cannot access 'PWNED2': No such file or directory ls: cannot access 'PWNED3': No such file or directory $ cat argv.txt ### rangeTotal ARGC=3 ARG[summary] ARG[:all] ARG[it's $(touch /tmp/parity/PWNED) `touch /tmp/parity/PWNED2` "q" ; touch /tmp/parity/PWNED3 ; #] ``` One argv element, byte-identical to the input, no canary created. A separate round-trip through `/bin/sh` with a tag carrying an embedded newline, a literal `rm -rf` line, a backtick pair and a trailing backslash came back byte for byte unchanged, and left the canary file it named alive. Every user-reachable interpolation is quoted: the tag in `dayTotals` (299), `rangeTotal` (352), and the date in `intervalsForDay` (318). `allIntervals` never puts the tag in the command at all. The only raw interpolations are the numeric bounds in finding 3, which no caller controls. **Parser parity.** Same harness, feeding one canned stdout to each old callback and to the matching parser, then comparing the module state against the return: ``` active {active=true,startEpoch=1756662533,tags=deep work} idle {active=nil,startEpoch=0,tags=} total 0:12:34 total-empty 0:00:00 tags {…} <-- differs, finding 2 days {1={date=2026-08-31,dow=Mon,seconds=754}} dayRows {1={at=17:48,…,tag=focus},2={…,tag=deep work},3={…,tag=(untagged)}} allRows {1={at=09:00,date=30/08,duration=2:00,tag=deep work}} allTotal 5:20 allTotal-empty 0:00 ``` Identical on every row but `tags`. The parsers are genuinely pure: they ran in a harness that never touched module state and never called `render`. **Structure.** `git diff --numstat main refactor/t3a-timew` reports `35 0 test.luau` and `204 0 timew/panel.luau`: 239 insertions, zero deletions, so no live comment was removed. Only those two files are modified; `git ls-tree -r` on the branch shows no `*.md`, no `secretspec.toml`, no `forge.sh`. The table sits at 185–370, immediately before `pokeBar()` at 373. `Timew` is added to the return table, which the host ignores, so no handler name, settings key, or IPC event changed. **Quirk coverage, since you asked what the asserts miss.** Blank lines, `(untagged)`, quoted multi-word tags, span-versus-prose, and empty output are all covered. Three gaps. The `timew tags` header and rule-width trick lives in the awk rather than the parser, so only finding 1 can reach it. The other two are parser asserts you could add today: `parseDayTotals` is never fed timew's prose day (`2026-08-30 Sun No filtered data`), which the `^(%S+)%s+(%S+)%s+(%S+)$` pattern *drops* rather than zeroes — the shell `case` is the only thing saving it, so an assert there documents why that `case` must never be deleted; and `parseRangeTotal` is asserted with `No filtered data` where timew actually prints `No filtered data found in the range …`. ## Answers **5a. `Timew.exec(cmd, cb)` or `Timew.start(tag)` / `Timew.stop()`? Keep `exec`, and do not add the typed pair in T3b either.** Inside `panel.luau` the pair would be clean — three call sites (599, 745, 1176), all of which would lose their `shQuote`. The reason not to is `bar.luau:142`, which builds the identical `"timew start " .. shQuote(…)` and `"timew stop"` with its own `shQuote` at line 34, and cannot reach `panel.Timew` because the plugin API forbids `require` between entries. A typed pair would therefore exist for one of the two callers and not the other: the tag's quoting moves inside the table for the panel and stays at the call site for the bar, which is two idioms for one operation instead of one. `exec` also mirrors bar's `runAsync(cmd .. " 2>&1", …)` exactly. If you ever want the typed pair it has to be written into both entries, and that is a decision about the no-require rule, not about this table. **5b. `walkMax` bounding the walk before the Luau tag filter: keep, and it is already documented.** The comment at 335–337 states the consequence precisely, and the pairing with `rangeTotal` is the honest mitigation: a truncated list still reports a true total, because the number comes from timew rather than from the rows. This is `main`'s behaviour, and preserving it is exactly the wave 1 contract. Please make sure T3b does not quietly "improve" it, since a rewire that changes what the list contains is drift wearing a refactor's clothes. Worth its own issue, and if it is ever fixed the fix is in timew, not in Luau: give the DOM walk a `timew get` filtered range so the bound applies after the tag, not before. **5c. Counts and bounds as parameters: right for `tag`, needless for the bounds.** See finding 3. Default `days`, `walkMax` to the module constants and keep the tag arguments. **Also asked, on over-engineering.** No `Timew` function is unused: all seven parsers feed a command function or an assert, and all eight command functions map to a known caller. `tagMatches` earns its place over `matchesFilter` for the reason in finding 5. There is no dead nil-callback path — every function calls `cb(…)` unconditionally, with no `if cb then` guard, which is the right call. The only indirection I would cut is the bounds.

yuris force-pushed refactor/t3a-timew from 175d69f1a2 to d39db8c727

2026-08-31 17:02:25 +00:00

Compare

yuris force-pushed refactor/t3a-timew from d39db8c727 to 38e891610d

2026-08-31 17:05:02 +00:00

Compare

yuris commented

2026-08-31 17:06:09 +00:00

Author

Owner

All five findings taken, plus both optional parser asserts from your quirk-coverage
note. Rebased onto e3d69aa, fixup pushed as 38e8916. Two deviations from your
prototype and one correction to the review, below.

Finding 1 — command strings are now values, and asserted

Taken as prototyped. Timew.tagArg (467) is extracted and exported, the duplicated
conditional is gone from both sites, and Timew.cmd.<name> exists for all eight.
The impure functions are now a runAsync, a timeout and a parse, nothing else.

I verified your module-globals claim independently before repeating it in a
committed comment, since the whole design rests on it:

$ luau env_probe.luau
module sees INJECTED = nil
main sees INJECTED   = from-main

Confirmed. A stub assigned in test.luau is invisible inside panel.luau, so
the strings had to become values rather than the consumer becoming stubbable.

Byte-identity, re-proved after the rebase and after the restructure. Same
harness shape you rebuilt: return block cut off both sides, tail concatenated
on, runAsync stubbed to record. Twenty-one invocations now — the eight sites,
the bound defaults, the tag-carrying ones with and without a filter, and each of
those again with the hostile tag. The new side drives the impure functions, not
the builders, so the timeout and the builder wiring are both under the diff.

$ diff out_old.txt out_new.txt && echo "PARITY OK"
PARITY OK

$ md5sum out_old.txt out_new.txt
ce815eda9a44377c25248a29c5dcb2fe  out_old.txt
ce815eda9a44377c25248a29c5dcb2fe  out_new.txt

Two of the twenty-one, quoted since they are the ones the finding is about:

04 dayTotals                timeout=15000
   for i in $(seq 0 13); do   d=$(date -d "-$i day" +%Y-%m-%d); e=$(date -d "$((1-i)) day" +%Y-%m-%d);   t=$(timew summary "$d" - "$e" 2>/dev/null | awk 'NF{l=$NF} END{print l}');   case "$t" in *:*:*) ;; *) t=0:00:00;; esac;   echo "$d $(date -d "$d" +%a) $t"; done
16 rangeTotal-tag           timeout=10000
   timew summary :all 'deep work' 2>/dev/null | awk 'NF{l=$NF} END{print l}'

Deviation 1: the bound defaults live in the builder, not the impure function.
Your sketch has walkMax = walkMax or ALL_WALK_MAX as the first line of
Timew.allIntervals. That leaves Timew.cmd.allIntervals() producing
[ "$i" -le nil ], so the frozen literal would have had to hard-code 200
supplied by the test rather than by the module constant — the assert would stop
being a check on the default. Putting the default in the builder makes the builder
total, keeps one place for the default, and lets the literal pin both:

assert(T.cmd.allIntervals() == [==[… [ "$i" -le 200 ]; do …]==])
assert(T.cmd.allIntervals(200) == T.cmd.allIntervals(), "an explicit 200 is the default")

Same for days (seq 0 13 in the frozen literal is DAYS - 1) and walkMax.

Deviation 2: Timew.cmd.exec exists too, so it is nine builders, not eight.
It is one concatenation and on its own it would be ceremony. It is in because it
makes "every command string in this table comes out of Timew.cmd" true with no
exception, and because the 2>&1 placement is exactly the class of thing you
asked to freeze — it is what carries a failure message to the toast.

On the offer to defer this to T3b: declined, and thank you for making it.
Landing Timew.cmd in T3b means the strings spend one commit unprotected in the
exact window where the old ones get deleted, which is the risk the finding names.
Cheaper here.

Non-vacuity. Frozen literals are worthless if they cannot fail, so each new
assert was mutation-checked on a scratch copy:

NR==3 -> NR==4                        -> test.luau:229
$NF -> $1                             -> test.luau:224
DAY_WALK_MAX -> 61                    -> test.luau:249
tagArg: drop the tag == "" arm        -> test.luau:278
parseTags: re-add the RECENT_MAX cap  -> test.luau:177

Each fails, at the intended assert.

One thing your prototype's shape does not survive contact with: the day-totals
string cannot go through string.format, because it is full of date(1)
specifiers (%Y, %m, %a) that a format eats. It is split around the tag and
concatenated instead.

Finding 2 — `parseTags` no-cap assert

Taken verbatim, at 177. The comment on parseTags now says the cap is the
caller's and that this assert is what enforces it, so the prose and the test
point at each other.

Finding 3 — bounds defaulted

Taken. days or DAYS, walkMax or DAY_WALK_MAX, walkMax or ALL_WALK_MAX, in
the builders per deviation 1. The tag parameters are untouched. Your reasoning
is right and I had it backwards in the original description: the purity argument
never reached these three, since they call runAsync.

Finding 4 — line numbers

Taken, and it was worse than you found. Both columns were wrong by the time I
fixed them: PR #2 moved main's fetch functions too, so the line on main
column — correct when you reviewed — is now also stale. The table is rebuilt with
both columns re-measured, a builder column added, and the two commits it is
relative to named, so the next reader can tell when it went stale.

The assert count was 20; it is now 43 across 129 added lines. Description says 43.

I made the same mistake once more while fixing it and want to flag the trap: I
first wrote matchesFilter's call sites into a code comment as 523, 820, 856,
which are main's numbers, not this branch's — the Timew table is inserted
above them and shifts all three. Then correcting that comment grew it by a line
and shifted them again. They are 813, 1110, 1146 on 38e8916, verified after
the final edit rather than before it.

Finding 5 — `matchesFilter` disposition

Timew.tagMatches wins; matchesFilter is deleted in T3b. Recorded in the
comment on tagMatches rather than only here, since that is where someone will
be standing when the question comes up. Not deleted now: it has three live call
sites and wave 1 only adds.

5a — `Timew.exec` stays a command string

Agreed with your ruling, and I checked the load-bearing part of it rather than
taking it: bar.luau:215 builds "timew stop" and "timew start " .. shQuote(…)
with its own shQuote at 99, and cannot reach panel.Timew because the API
forbids require between entries. A typed pair would serve one caller of two.
The reasoning is now in the comment above Timew.exec, including the instruction
that T3b should not add one either, so it does not get re-litigated by someone who
only sees the panel side.

Rebase

Conflicts were two, both purely additive: the export table, and the tail of
test.luau. Your Fmt/Parse/Query asserts and PR #1's bar asserts are untouched;
mine are appended below them.

The real overlap was that my parsers reused this file's older free helpers for
jobs PR #2's pure core now does. Rather than ship two spellings, Timew.parse*
now calls Parse.trim, Parse.unquote, Parse.clock, Parse.interval and
Fmt.span. I diffed each pair over a shared corpus before switching, because
"looks the same" is not the standard for something in the quoting path:

194 comparisons, 0 differences
PARSE EQUIVALENT

Corpus was the empty string, whitespace-only, unterminated and empty quotes,
apostrophes, malformed clocks (1:2:3, ::), a non-breaking space, embedded
newlines, ISO durations including bare P and PT, and fractional and negative
second counts. No pair differed, so none was kept back.

Then the end-to-end check, one canned stdout to each old callback and to the
matching parser, comparing module state against the return, plus exec against
run on exit 0, 1 and 127:

NOTE tags: old capped 6 -> 4 at RECENT_MAX; new returns all 6
85 comparisons, 0 differences
STATE EQUIVALENT (tags cap noted above, by design)

tags is still the only difference, still deliberate, and now asserted.

Gate

$ nix shell nixpkgs#luau -c luau test.luau
ok
exit=0

$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings
exit=0

$ git diff --numstat origin/main...HEAD
129     0       test.luau
291     0       timew/panel.luau

Still zero deletions, still only those two files, no excluded file committed.

All five findings taken, plus both optional parser asserts from your quirk-coverage note. Rebased onto `e3d69aa`, fixup pushed as `38e8916`. Two deviations from your prototype and one correction to the review, below. ## Finding 1 — command strings are now values, and asserted Taken as prototyped. `Timew.tagArg` (467) is extracted and exported, the duplicated conditional is gone from both sites, and `Timew.cmd.<name>` exists for all eight. The impure functions are now a `runAsync`, a timeout and a parse, nothing else. I verified your module-globals claim independently before repeating it in a committed comment, since the whole design rests on it: ``` $ luau env_probe.luau module sees INJECTED = nil main sees INJECTED = from-main ``` Confirmed. A stub assigned in `test.luau` is invisible inside `panel.luau`, so the strings had to become values rather than the consumer becoming stubbable. **Byte-identity, re-proved after the rebase and after the restructure.** Same harness shape you rebuilt: `return` block cut off both sides, tail concatenated on, `runAsync` stubbed to record. Twenty-one invocations now — the eight sites, the bound defaults, the tag-carrying ones with and without a filter, and each of those again with the hostile tag. The new side drives the *impure* functions, not the builders, so the timeout and the builder wiring are both under the diff. ``` $ diff out_old.txt out_new.txt && echo "PARITY OK" PARITY OK $ md5sum out_old.txt out_new.txt ce815eda9a44377c25248a29c5dcb2fe out_old.txt ce815eda9a44377c25248a29c5dcb2fe out_new.txt ``` Two of the twenty-one, quoted since they are the ones the finding is about: ``` 04 dayTotals timeout=15000 for i in $(seq 0 13); do d=$(date -d "-$i day" +%Y-%m-%d); e=$(date -d "$((1-i)) day" +%Y-%m-%d); t=$(timew summary "$d" - "$e" 2>/dev/null | awk 'NF{l=$NF} END{print l}'); case "$t" in *:*:*) ;; *) t=0:00:00;; esac; echo "$d $(date -d "$d" +%a) $t"; done 16 rangeTotal-tag timeout=10000 timew summary :all 'deep work' 2>/dev/null | awk 'NF{l=$NF} END{print l}' ``` **Deviation 1: the bound defaults live in the builder, not the impure function.** Your sketch has `walkMax = walkMax or ALL_WALK_MAX` as the first line of `Timew.allIntervals`. That leaves `Timew.cmd.allIntervals()` producing `[ "$i" -le nil ]`, so the frozen literal would have had to hard-code `200` supplied by the test rather than by the module constant — the assert would stop being a check on the default. Putting the default in the builder makes the builder total, keeps one place for the default, and lets the literal pin both: ```lua assert(T.cmd.allIntervals() == [==[… [ "$i" -le 200 ]; do …]==]) assert(T.cmd.allIntervals(200) == T.cmd.allIntervals(), "an explicit 200 is the default") ``` Same for `days` (`seq 0 13` in the frozen literal *is* `DAYS - 1`) and `walkMax`. **Deviation 2: `Timew.cmd.exec` exists too, so it is nine builders, not eight.** It is one concatenation and on its own it would be ceremony. It is in because it makes "every command string in this table comes out of `Timew.cmd`" true with no exception, and because the ` 2>&1` placement is exactly the class of thing you asked to freeze — it is what carries a failure message to the toast. **On the offer to defer this to T3b: declined, and thank you for making it.** Landing `Timew.cmd` in T3b means the strings spend one commit unprotected in the exact window where the old ones get deleted, which is the risk the finding names. Cheaper here. **Non-vacuity.** Frozen literals are worthless if they cannot fail, so each new assert was mutation-checked on a scratch copy: ``` NR==3 -> NR==4 -> test.luau:229 $NF -> $1 -> test.luau:224 DAY_WALK_MAX -> 61 -> test.luau:249 tagArg: drop the tag == "" arm -> test.luau:278 parseTags: re-add the RECENT_MAX cap -> test.luau:177 ``` Each fails, at the intended assert. One thing your prototype's shape does not survive contact with: the day-totals string cannot go through `string.format`, because it is full of `date(1)` specifiers (`%Y`, `%m`, `%a`) that a format eats. It is split around the tag and concatenated instead. ## Finding 2 — `parseTags` no-cap assert Taken verbatim, at 177. The comment on `parseTags` now says the cap is the caller's and that this assert is what enforces it, so the prose and the test point at each other. ## Finding 3 — bounds defaulted Taken. `days or DAYS`, `walkMax or DAY_WALK_MAX`, `walkMax or ALL_WALK_MAX`, in the builders per deviation 1. The `tag` parameters are untouched. Your reasoning is right and I had it backwards in the original description: the purity argument never reached these three, since they call `runAsync`. ## Finding 4 — line numbers Taken, and it was worse than you found. Both columns were wrong by the time I fixed them: PR #2 moved `main`'s fetch functions too, so the `line on main` column — correct when you reviewed — is now also stale. The table is rebuilt with both columns re-measured, a builder column added, and the two commits it is relative to named, so the next reader can tell when it went stale. The assert count was 20; it is now 43 across 129 added lines. Description says 43. I made the same mistake once more while fixing it and want to flag the trap: I first wrote `matchesFilter`'s call sites into a code comment as `523, 820, 856`, which are `main`'s numbers, not this branch's — the `Timew` table is inserted above them and shifts all three. Then correcting that comment grew it by a line and shifted them again. They are `813, 1110, 1146` on `38e8916`, verified after the final edit rather than before it. ## Finding 5 — `matchesFilter` disposition `Timew.tagMatches` wins; `matchesFilter` is deleted in T3b. Recorded in the comment on `tagMatches` rather than only here, since that is where someone will be standing when the question comes up. Not deleted now: it has three live call sites and wave 1 only adds. ## 5a — `Timew.exec` stays a command string Agreed with your ruling, and I checked the load-bearing part of it rather than taking it: `bar.luau:215` builds `"timew stop"` and `"timew start " .. shQuote(…)` with its own `shQuote` at 99, and cannot reach `panel.Timew` because the API forbids `require` between entries. A typed pair would serve one caller of two. The reasoning is now in the comment above `Timew.exec`, including the instruction that T3b should not add one either, so it does not get re-litigated by someone who only sees the panel side. ## Rebase Conflicts were two, both purely additive: the export table, and the tail of `test.luau`. Your Fmt/Parse/Query asserts and PR #1's bar asserts are untouched; mine are appended below them. The real overlap was that my parsers reused this file's older free helpers for jobs PR #2's pure core now does. Rather than ship two spellings, `Timew.parse*` now calls `Parse.trim`, `Parse.unquote`, `Parse.clock`, `Parse.interval` and `Fmt.span`. I diffed each pair over a shared corpus before switching, because "looks the same" is not the standard for something in the quoting path: ``` 194 comparisons, 0 differences PARSE EQUIVALENT ``` Corpus was the empty string, whitespace-only, unterminated and empty quotes, apostrophes, malformed clocks (`1:2:3`, `::`), a non-breaking space, embedded newlines, ISO durations including bare `P` and `PT`, and fractional and negative second counts. No pair differed, so none was kept back. Then the end-to-end check, one canned stdout to each old callback and to the matching parser, comparing module state against the return, plus `exec` against `run` on exit 0, 1 and 127: ``` NOTE tags: old capped 6 -> 4 at RECENT_MAX; new returns all 6 85 comparisons, 0 differences STATE EQUIVALENT (tags cap noted above, by design) ``` `tags` is still the only difference, still deliberate, and now asserted. ## Gate ``` $ nix shell nixpkgs#luau -c luau test.luau ok exit=0 $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings exit=0 $ git diff --numstat origin/main...HEAD 129 0 test.luau 291 0 timew/panel.luau ``` Still zero deletions, still only those two files, no excluded file committed.

yuris merged commit ecf52e548c into main

2026-08-31 17:06:17 +00:00

yuris deleted branch refactor/t3a-timew

2026-08-31 17:06:17 +00:00

yuris referenced this pull request from a commit

2026-08-31 17:06:19 +00:00

refactor(panel): add Timew command layer (#3)

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

yuris/noctalia-timew!3

No description provided.

Rows
Columns

refactor(panel): add Timew command layer #3

refactor(panel): add a Timew command layer

Coverage: all eight command sites

Verified

Parity, and this time it is reproducible

Decisions

Why the command strings are frozen literals

Findings

1. The parity evidence cannot be committed as written, and after T3b nothing can re-derive it — timew/panel.luau:279, :351

2. parseTags drops the RECENT_MAX cap, and nothing tests it — timew/panel.luau:222 vs :462

3. days and walkMax are parameters with exactly one possible argument — timew/panel.luau:298, :316, :337

4. Every line number in the coverage table points at the wrong code

5. Give matchesFilter a disposition before it becomes permanent — timew/panel.luau:148 and :192

Verified

Answers

Finding 1 — command strings are now values, and asserted

Finding 2 — parseTags no-cap assert

Finding 3 — bounds defaulted

Finding 4 — line numbers

Finding 5 — matchesFilter disposition

5a — Timew.exec stays a command string

Rebase

Gate

1. The parity evidence cannot be committed as written, and after T3b nothing can re-derive it — `timew/panel.luau:279`, `:351`

2. `parseTags` drops the `RECENT_MAX` cap, and nothing tests it — `timew/panel.luau:222` vs `:462`

3. `days` and `walkMax` are parameters with exactly one possible argument — `timew/panel.luau:298`, `:316`, `:337`

5. Give `matchesFilter` a disposition before it becomes permanent — `timew/panel.luau:148` and `:192`

Finding 2 — `parseTags` no-cap assert

Finding 5 — `matchesFilter` disposition

5a — `Timew.exec` stays a command string