yuris/noctalia-timew
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(panel): swap the call sites onto the pure core, key the last literals #7

Merged
yuris merged 2 commits from refactor/t4-swap into main 2026-08-31 19:17:44 +00:00
Conversation 2 Commits 2 Files changed 3 +255 -147
yuris commented 2026-08-31 18:53:53 +00:00
Owner
Copy link

Wave 3, second and final PR. Two passes on the same call sites: point every
caller at the pure core that landed in wave 1 and delete the duplicates, then
move the last user-visible literals into translation keys.

Rendered output is unchanged. Proof below.

Part 1 — call sites onto the pure core

deleted from panel.luau replaced by call sites moved
tagColor (7 lines) Query.tagColor(tag, TAG_COLORS) 1 — the row dot's fill
trim (3 lines) Parse.trim 2 — startTag, setFilter
formatElapsed (7 lines) Fmt.clock 1 — the live clock in render
formatSpan (7 lines) Fmt.span 1 — the day total in render
elapsed (6 lines) inlined, see below 1
inline dayTitle expression Fmt.dayLabel(entry, index) 1
inline barFill / letterColor Query.barRoles(mode, filter, seconds, isSelected) 1
inline window loop in activityRows Query.window 1
inline math.ceil(matching / ROWS_MAX) Query.pageCount 2 — gotoPage, render
inline math.floor(listOffset / ROWS_MAX) Query.pageOf 3 — onPagePrev, onPageNext, render
inline firstShown / lastShown Query.pageStrip 1

The last four rows are past the brief's list. They were the reason to look:
Query.window, pageCount, pageOf and pageStrip had no production caller
at all after wave 1 — four tested functions the panel was not using while it
carried its own copy of each. Leaving them would have been shipping the pure
core as dead code.

Grep, after each deletion

$ grep -nE '(^|[^.[:alnum:]_])(trim|tagColor|formatElapsed|formatSpan|elapsed)\(' timew/panel.luau
$

Nothing. Parse.trim and the other qualified names are excluded by the
leading-. guard, so this is specifically "a bare call to a helper that no
longer exists". Also checked: grep -c 'getfenv\|setfenv' timew/panel.luau → 0.

shQuote — one definition, and which

shQuote is not duplicated by the pure core; Parse has no quoting function,
so there was nothing to delete. It stays a module local rather than moving into
Parse, for two reasons: it is not text → data, and Timew.tagArg already
exports the whole injection surface, which is where test.luau's seven quoting
asserts point. It has three callers — Timew.tagArg, Timew.cmd.intervalsForDay,
and startTag, which builds timew start <tag> itself because bar.luau:215
builds the identical string with its own copy and the plugin API forbids
require between entries. That is the same reasoning PR #6 recorded on
Timew.exec, unchanged.

elapsed — the third answer

Neither "stays" nor "moves to the pure core". It is gone, inlined at its only
call site:

local clockText = if not active
    then noctalia.tr("panel.clock_idle")
    elseif elapsedText ~= "" then elapsedText
    else Fmt.clock(os.time() - startEpoch)

elapsed() was if not active then return 0 end; return os.time() - startEpoch.
Its guard is dead here: the not active branch above has already answered, so
the function reduced to one subtraction. The pure-core version would have been
Query.elapsed(active, startEpoch, now) — three threaded arguments to express
os.time() - startEpoch, and a function with nothing a test could catch it
getting wrong. Fmt.clock still clamps negatives, so a start epoch in the
future reads 0:00 exactly as before. The comment on update() that promised
"elapsed() is recomputed from the start epoch" moved to the call site rather
than being left pointing at a function that no longer exists.

Part 2 — literals → keys

literal key value
"--" in Fmt.dayLabel panel.no_day --
"--:--" (idle clock) panel.clock_idle --:--
"(untagged)" shown on a row / the subject line panel.untagged (untagged)
filter .. " ✕" panel.filter_chip {tag} ✕
`{listOffset+1}-{listOffset+#shown} / {matching}` panel.page_range {first}-{last} / {total}

Fmt.dayLabel now returns ("panel.no_day", true) for a nil entry instead of
("--", false), so all three of its answers go through the same isKey route
and the caller has no special case. Its unit assert moved with it.

After the swap the only string literals left in a text / placeholder /
tooltip prop are the two chevrons:

$ grep -nE '(text|placeholder|tooltip) = "' timew/panel.luau
1235:					text = "‹",
1260:					text = "›",

The (untagged) trap, and how it is handled

One string was doing two jobs. It is what a row displays, and it is also:

  • compared with ~= in filterFromRow, to refuse a filter nobody can act on;
  • fed to Timew.tagMatches as a row tag when the search runs;
  • hashed by Query.tagColor for the row's colour dot;
  • published by bar.luau:180 into the timew.tags shell state, which
    adoptState reads straight back into the panel.

That last one settles it. It is a wire value between two plugin entries that
cannot require each other, so it has to be the same bytes in every locale.
Translating it breaks the filter, the search and the dot's colour stability, and
does so silently — the panel would look right in English and stop refusing the
untagged row in Spanish.

So the two jobs are split. The sentinel is a module constant, UNTAGGED, and it
is what Parse.interval, Timew.parseActive and filterFromRow use. Display
goes through one function:

local function tagLabel(tag)
	return if tag == UNTAGGED then noctalia.tr("panel.untagged") else tag
end

with three call sites: the row button's text, that button's panel.filter_by
tooltip, and the tracking subject line. panel.untagged resolves to
(untagged) in en.json, so English is byte-identical; a translator changing
it changes only what is on screen.

Chevrons and ✕

‹ and › stay literals. They are the button's shape, not its text: a key here
has exactly one legitimate use (an RTL locale flipping them) against one likely
misuse (a translator writing a word into a five-button strip pinned to
RIGHT_W, pushing the page numbers off the panel). The prose naming the action
is already a key, on the tooltip of the same button — panel.newer and
panel.older. The rule applied: a standalone glyph is a shape and stays; a
string that composes chrome with user data is a template and gets a key.

Which is why the ✕ went the other way. The glyph needs no translating, but the
chip is tag + spacing + glyph, and that ordering and spacing are prose — an
RTL locale wants the ✕ on the other side. panel.filter_chip is {tag} ✕, so
the composition is translatable and the glyph inside it is not something a
translator has to retype.

Not translated, on purpose

"0:00" and "0:00:00" — the zero defaults in Timew.parseTotal,
Timew.parseRangeTotal and the day total. They are values in timew's own clock
format, sitting in the same label as real durations from Fmt.span. A locale
that "translated" one would put a non-clock string where the panel reads h:mm.

Parity evidence

Throwaway harness (not committed, /tmp/parity/harness.luau): requires
git show main:timew/panel.luau and this branch's copy as two modules, stubs
one host into each module's own globals table, pins os.time() so the tracking
clock is deterministic, and resolves noctalia.tr against the real en.json
so a key's resolved English text is compared, not the key. It then runs one
identical script against both and diffs the flattened UI trees — one line per
node, kind plus every prop sorted by name, under an index path, so label text,
colour role, handler name, tooltip and every dimension are all in the
comparison.

states=30 nodes compared=3407 differences=0
PARITY OK

The 30 states: open with nothing landed; recent chips; today's total; the 14
day bars; today's intervals (which include an untagged row and a multi-word
tag); tracking a plain tag; tracking a multi-word tag; the refetch after a
start; tracking with no tag; back to idle; yesterday selected (named-day label);
a dated day with no intervals; back to today; clicking the untagged row;
filtered day scope; filtered all-time one page; all-time with 37 matches at
page 1 of 5; pages 2, 3 (middle), 5 (last, short), 4, 1; all-time unfiltered
with the untagged row in the all-time lead column; back to day scope; a filter
nothing matches; cleared; a case-insensitive search submit; a draft in the tag
field on page 2; a recent chip starting its tag 60 s in; a second tick.

The harness was checked against itself before the result was trusted. Two
deliberate mutations of the branch copy:

  • TAG_COLORS[1] primary → error — 11 differences, PARITY FAILED
  • panel.clock_idle → panel.no_day on the idle clock, i.e. resolved English
    --:-- → -- — 24 differences, PARITY FAILED

Both restored byte-identically (cmp clean), back to 0.

Tests

test.luau goes 615 → 686 lines. Updated: the formatSpan and formatElapsed
loops now run the surviving definitions only, the bar/panel drift section
compares bar's copies against the single definition panel is left with,
getfenv(panel.formatElapsed) became getfenv(panel.Fmt.clock), and
Fmt.dayLabel(nil) is pinned to the key. The export table lost
formatElapsed and formatSpan.

Thirteen new asserts, each proved live by mutating timew/panel.luau and
checking the suite dies on that exact line:

killed 66   Fmt.dayLabel answers the empty state with a key
killed 577  render feeds the day label into the scope button
killed 642  tagLabel translates the sentinel before it is shown
killed 643  the untagged row shows the untagged key, not just some key
killed 649  filterFromRow refuses the sentinel
killed 654  filterFromRow still filters by a real tag
killed 667  Query.window caps the page at ROWS_MAX
killed 668  the page range is not a string built in the panel
killed 669  the page range is that exact key
killed 672  gotoPage reaches the last page
killed 679  render passes the selected index to Fmt.dayLabel
killed 684  loadDay selects the day it was asked for
killed 685  the empty list is the nothing-tracked key
--- final state
panel.luau byte-identical to pristine
ok

Two assert pairs were reordered so neither hides the other under mutation: the
(untagged)-never-on-screen check runs before the row-text check, and the
"the built string is gone" check runs before "and the key is there".

The sentinel asserts are the ones to keep. The stubbed tr hands back the key,
which is exactly how a key is told from a literal on the rendered tree: a row
still reading (untagged) down there never went through tr and would stay
English in every locale. And onIpc("row", 1) on the untagged row must leave
the trend caption unfiltered, with the real-tag row below it as the control —
without that control, a dead click path would pass just as well.

Gate

$ nix shell nixpkgs#luau -c luau test.luau
ok
$ noctalia plugins lint timew
0 errors, 0 warnings
$ git diff --stat main...HEAD --name-only
test.luau
timew/panel.luau
timew/translations/en.json
$ grep -c 'getfenv\|setfenv' timew/panel.luau
0

Note on gate 2: noctalia plugins lint cross-checks declared settings against
getConfig() calls and nothing else — it does not parse Luau and does not
validate translation keys. I confirmed that by appending a syntax error and
watching it still report 0 errors. The Luau parse is covered by gate 1, which
requires panel.luau.

Not done, on purpose: render is not split into region builders, there is no
viewModel, bar.luau is untouched, and no layout, spacing, colour or
geometry changed.

What to screenshot after merge

The parity harness covers the tree, not the pixels. These are the states where
a rendered difference could still hide:

  1. Panel open, idle, before the first fetch lands — the clock reads --:--
    and the left scope button reads --. Both are keys now; a missing key would
    render as the key name.
  2. Idle with history — the scope button reads Today, the total to its
    right, bars unfiltered.
  3. Tracking a multi-word tag — the clock ticks, the subject line shows the
    tag as plain text at SUBJECT_H, the card does not resize when it starts.
  4. A day whose intervals include an untagged one — that row shows
    (untagged), its dot has the same colour it had before this PR, and its
    duration is right-aligned with the header total.
  5. Click the untagged row — nothing must happen: no chip, no bar change.
    Then click the row below it — the chip appears and reads <tag> ✕ with the
    same two spaces before the glyph.
  6. The filter chip — hover for the Clear tag filter tooltip, click the ✕,
    confirm the chip clears and the list re-lists.
  7. Filtered, All time — bars carrying the tag at primary/0.5, the selected
    bar at full primary, day letters under the lit bars in primary. This is
    the Query.barRoles swap; it is the one place a colour role could have moved.
  8. A tag with more than 8 matches all-time — the pager appears. Check page
    1, a middle page, and the last page: the ‹/› ends, the current page
    tinted, and the range label at the right reading 9-16 / 37. Confirm the
    strip slides rather than growing, and that the range label does not wrap.
  9. A day with nothing tracked — Nothing tracked under a dated scope button
    like Sat 29/08.
  10. The bar widget beside the panel while tracking — both clocks showing the
    same second. elapsed() was inlined; this is where that would show.
Wave 3, second and final PR. Two passes on the same call sites: point every caller at the pure core that landed in wave 1 and delete the duplicates, then move the last user-visible literals into translation keys. Rendered output is unchanged. Proof below. ## Part 1 — call sites onto the pure core | deleted from `panel.luau` | replaced by | call sites moved | | --- | --- | --- | | `tagColor` (7 lines) | `Query.tagColor(tag, TAG_COLORS)` | 1 — the row dot's fill | | `trim` (3 lines) | `Parse.trim` | 2 — `startTag`, `setFilter` | | `formatElapsed` (7 lines) | `Fmt.clock` | 1 — the live clock in `render` | | `formatSpan` (7 lines) | `Fmt.span` | 1 — the day total in `render` | | `elapsed` (6 lines) | inlined, see below | 1 | | inline `dayTitle` expression | `Fmt.dayLabel(entry, index)` | 1 | | inline `barFill` / `letterColor` | `Query.barRoles(mode, filter, seconds, isSelected)` | 1 | | inline window loop in `activityRows` | `Query.window` | 1 | | inline `math.ceil(matching / ROWS_MAX)` | `Query.pageCount` | 2 — `gotoPage`, `render` | | inline `math.floor(listOffset / ROWS_MAX)` | `Query.pageOf` | 3 — `onPagePrev`, `onPageNext`, `render` | | inline `firstShown` / `lastShown` | `Query.pageStrip` | 1 | The last four rows are past the brief's list. They were the reason to look: `Query.window`, `pageCount`, `pageOf` and `pageStrip` had no production caller at all after wave 1 — four tested functions the panel was not using while it carried its own copy of each. Leaving them would have been shipping the pure core as dead code. ### Grep, after each deletion ``` $ grep -nE '(^|[^.[:alnum:]_])(trim|tagColor|formatElapsed|formatSpan|elapsed)\(' timew/panel.luau $ ``` Nothing. `Parse.trim` and the other qualified names are excluded by the leading-`.` guard, so this is specifically "a bare call to a helper that no longer exists". Also checked: `grep -c 'getfenv\|setfenv' timew/panel.luau` → `0`. ### `shQuote` — one definition, and which `shQuote` is not duplicated by the pure core; `Parse` has no quoting function, so there was nothing to delete. It stays a module local rather than moving into `Parse`, for two reasons: it is not text → data, and `Timew.tagArg` already exports the whole injection surface, which is where `test.luau`'s seven quoting asserts point. It has three callers — `Timew.tagArg`, `Timew.cmd.intervalsForDay`, and `startTag`, which builds `timew start <tag>` itself because `bar.luau:215` builds the identical string with its own copy and the plugin API forbids `require` between entries. That is the same reasoning PR #6 recorded on `Timew.exec`, unchanged. ### `elapsed` — the third answer Neither "stays" nor "moves to the pure core". It is gone, inlined at its only call site: ```lua local clockText = if not active then noctalia.tr("panel.clock_idle") elseif elapsedText ~= "" then elapsedText else Fmt.clock(os.time() - startEpoch) ``` `elapsed()` was `if not active then return 0 end; return os.time() - startEpoch`. Its guard is dead here: the `not active` branch above has already answered, so the function reduced to one subtraction. The pure-core version would have been `Query.elapsed(active, startEpoch, now)` — three threaded arguments to express `os.time() - startEpoch`, and a function with nothing a test could catch it getting wrong. `Fmt.clock` still clamps negatives, so a start epoch in the future reads `0:00` exactly as before. The comment on `update()` that promised "elapsed() is recomputed from the start epoch" moved to the call site rather than being left pointing at a function that no longer exists. ## Part 2 — literals → keys | literal | key | value | | --- | --- | --- | | `"--"` in `Fmt.dayLabel` | `panel.no_day` | `--` | | `"--:--"` (idle clock) | `panel.clock_idle` | `--:--` | | `"(untagged)"` shown on a row / the subject line | `panel.untagged` | `(untagged)` | | `filter .. " ✕"` | `panel.filter_chip` | `{tag} ✕` | | `` `{listOffset+1}-{listOffset+#shown} / {matching}` `` | `panel.page_range` | `{first}-{last} / {total}` | `Fmt.dayLabel` now returns `("panel.no_day", true)` for a nil entry instead of `("--", false)`, so all three of its answers go through the same `isKey` route and the caller has no special case. Its unit assert moved with it. After the swap the only string literals left in a `text` / `placeholder` / `tooltip` prop are the two chevrons: ``` $ grep -nE '(text|placeholder|tooltip) = "' timew/panel.luau 1235: text = "‹", 1260: text = "›", ``` ### The `(untagged)` trap, and how it is handled One string was doing two jobs. It is what a row displays, and it is also: - compared with `~=` in `filterFromRow`, to refuse a filter nobody can act on; - fed to `Timew.tagMatches` as a row tag when the search runs; - hashed by `Query.tagColor` for the row's colour dot; - **published by `bar.luau:180` into the `timew.tags` shell state**, which `adoptState` reads straight back into the panel. That last one settles it. It is a wire value between two plugin entries that cannot `require` each other, so it has to be the same bytes in every locale. Translating it breaks the filter, the search and the dot's colour stability, and does so silently — the panel would look right in English and stop refusing the untagged row in Spanish. So the two jobs are split. The sentinel is a module constant, `UNTAGGED`, and it is what `Parse.interval`, `Timew.parseActive` and `filterFromRow` use. Display goes through one function: ```lua local function tagLabel(tag) return if tag == UNTAGGED then noctalia.tr("panel.untagged") else tag end ``` with three call sites: the row button's text, that button's `panel.filter_by` tooltip, and the tracking subject line. `panel.untagged` resolves to `(untagged)` in `en.json`, so English is byte-identical; a translator changing it changes only what is on screen. ### Chevrons and ✕ `‹` and `›` stay literals. They are the button's shape, not its text: a key here has exactly one legitimate use (an RTL locale flipping them) against one likely misuse (a translator writing a word into a five-button strip pinned to `RIGHT_W`, pushing the page numbers off the panel). The prose naming the action is already a key, on the tooltip of the same button — `panel.newer` and `panel.older`. The rule applied: **a standalone glyph is a shape and stays; a string that composes chrome with user data is a template and gets a key.** Which is why the ✕ went the other way. The glyph needs no translating, but the chip is `tag` + spacing + glyph, and that ordering and spacing are prose — an RTL locale wants the ✕ on the other side. `panel.filter_chip` is `{tag} ✕`, so the composition is translatable and the glyph inside it is not something a translator has to retype. ### Not translated, on purpose `"0:00"` and `"0:00:00"` — the zero defaults in `Timew.parseTotal`, `Timew.parseRangeTotal` and the day total. They are values in timew's own clock format, sitting in the same label as real durations from `Fmt.span`. A locale that "translated" one would put a non-clock string where the panel reads h:mm. ## Parity evidence Throwaway harness (not committed, `/tmp/parity/harness.luau`): requires `git show main:timew/panel.luau` and this branch's copy as two modules, stubs one host into each module's own globals table, pins `os.time()` so the tracking clock is deterministic, and resolves `noctalia.tr` against the real `en.json` so a key's resolved English text is compared, not the key. It then runs one identical script against both and diffs the flattened UI trees — one line per node, `kind` plus every prop sorted by name, under an index path, so label text, colour role, handler name, tooltip and every dimension are all in the comparison. ``` states=30 nodes compared=3407 differences=0 PARITY OK ``` The 30 states: open with nothing landed; recent chips; today's total; the 14 day bars; today's intervals (which include an untagged row and a multi-word tag); tracking a plain tag; tracking a multi-word tag; the refetch after a start; tracking with no tag; back to idle; yesterday selected (named-day label); a dated day with no intervals; back to today; clicking the untagged row; filtered day scope; filtered all-time one page; all-time with 37 matches at page 1 of 5; pages 2, 3 (middle), 5 (last, short), 4, 1; all-time unfiltered with the untagged row in the all-time lead column; back to day scope; a filter nothing matches; cleared; a case-insensitive search submit; a draft in the tag field on page 2; a recent chip starting its tag 60 s in; a second tick. The harness was checked against itself before the result was trusted. Two deliberate mutations of the branch copy: - `TAG_COLORS[1]` `primary` → `error` — **11 differences**, `PARITY FAILED` - `panel.clock_idle` → `panel.no_day` on the idle clock, i.e. resolved English `--:--` → `--` — **24 differences**, `PARITY FAILED` Both restored byte-identically (`cmp` clean), back to 0. ## Tests `test.luau` goes 615 → 686 lines. Updated: the `formatSpan` and `formatElapsed` loops now run the surviving definitions only, the bar/panel drift section compares bar's copies against the single definition panel is left with, `getfenv(panel.formatElapsed)` became `getfenv(panel.Fmt.clock)`, and `Fmt.dayLabel(nil)` is pinned to the key. The export table lost `formatElapsed` and `formatSpan`. Thirteen new asserts, each proved live by mutating `timew/panel.luau` and checking the suite dies on that exact line: ``` killed 66 Fmt.dayLabel answers the empty state with a key killed 577 render feeds the day label into the scope button killed 642 tagLabel translates the sentinel before it is shown killed 643 the untagged row shows the untagged key, not just some key killed 649 filterFromRow refuses the sentinel killed 654 filterFromRow still filters by a real tag killed 667 Query.window caps the page at ROWS_MAX killed 668 the page range is not a string built in the panel killed 669 the page range is that exact key killed 672 gotoPage reaches the last page killed 679 render passes the selected index to Fmt.dayLabel killed 684 loadDay selects the day it was asked for killed 685 the empty list is the nothing-tracked key --- final state panel.luau byte-identical to pristine ok ``` Two assert pairs were reordered so neither hides the other under mutation: the `(untagged)`-never-on-screen check runs before the row-text check, and the "the built string is gone" check runs before "and the key is there". The sentinel asserts are the ones to keep. The stubbed `tr` hands back the key, which is exactly how a key is told from a literal on the rendered tree: a row still reading `(untagged)` down there never went through `tr` and would stay English in every locale. And `onIpc("row", 1)` on the untagged row must leave the trend caption unfiltered, with the real-tag row below it as the control — without that control, a dead click path would pass just as well. ## Gate ``` $ nix shell nixpkgs#luau -c luau test.luau ok $ noctalia plugins lint timew 0 errors, 0 warnings $ git diff --stat main...HEAD --name-only test.luau timew/panel.luau timew/translations/en.json $ grep -c 'getfenv\|setfenv' timew/panel.luau 0 ``` Note on gate 2: `noctalia plugins lint` cross-checks declared settings against `getConfig()` calls and nothing else — it does not parse Luau and does not validate translation keys. I confirmed that by appending a syntax error and watching it still report `0 errors`. The Luau parse is covered by gate 1, which `require`s `panel.luau`. Not done, on purpose: `render` is not split into region builders, there is no `viewModel`, `bar.luau` is untouched, and no layout, spacing, colour or geometry changed. ## What to screenshot after merge The parity harness covers the tree, not the pixels. These are the states where a rendered difference could still hide: 1. **Panel open, idle, before the first fetch lands** — the clock reads `--:--` and the left scope button reads `--`. Both are keys now; a missing key would render as the key name. 2. **Idle with history** — the scope button reads `Today`, the total to its right, bars unfiltered. 3. **Tracking a multi-word tag** — the clock ticks, the subject line shows the tag as plain text at `SUBJECT_H`, the card does not resize when it starts. 4. **A day whose intervals include an untagged one** — that row shows `(untagged)`, its dot has the same colour it had before this PR, and its duration is right-aligned with the header total. 5. **Click the untagged row** — nothing must happen: no chip, no bar change. Then click the row below it — the chip appears and reads `<tag> ✕` with the same two spaces before the glyph. 6. **The filter chip** — hover for the `Clear tag filter` tooltip, click the ✕, confirm the chip clears and the list re-lists. 7. **Filtered, All time** — bars carrying the tag at `primary/0.5`, the selected bar at full `primary`, day letters under the lit bars in `primary`. This is the `Query.barRoles` swap; it is the one place a colour role could have moved. 8. **A tag with more than 8 matches all-time** — the pager appears. Check page 1, a middle page, and the last page: the `‹`/`›` ends, the current page tinted, and the range label at the right reading `9-16 / 37`. Confirm the strip slides rather than growing, and that the range label does not wrap. 9. **A day with nothing tracked** — `Nothing tracked` under a dated scope button like `Sat 29/08`. 10. **The bar widget beside the panel while tracking** — both clocks showing the same second. `elapsed()` was inlined; this is where that would show. 
Wave 1 landed Fmt/Parse/Query beside the helpers they replace; this points
every caller at them and deletes the duplicates. tagColor, trim, formatElapsed
and formatSpan are gone, along with the inline copies of the list window, the
pager arithmetic, the day title and the bar roles. shQuote stays as the one
shell-quoting definition in the file: it is not text -> data, and startTag
still builds "timew start <tag>" itself because bar.luau duplicates that string
and cannot require across entries. elapsed() went with the swap rather than
into the pure core -- its `not active` guard was dead at its only call site,
which sits inside the branch that already tested it.

Every user-visible string is now a translation key. The trap is "(untagged)":
it is both what a row displays and what filterFromRow compares with `~=`, what
Timew.tagMatches searches, what Query.tagColor hashes, and what bar.luau
publishes into `timew.tags`. Translating it would break the filter in every
non-English locale, so the sentinel stays as the UNTAGGED constant and only its
display goes through tagLabel -> panel.untagged. The chevrons stay literals:
they are the button's shape, not its text, and the prose naming the action is
already a key on the same button's tooltip.

Rendered output is unchanged: a throwaway harness drove main's panel.luau and
this one under one stubbed host over 30 states, comparing 3407 flattened nodes
-- every label text, colour role, handler name and dimension -- with zero
differences.
yuris reviewed 2026-08-31 19:09:01 +00:00
yuris left a comment
Copy link

Verdict: APPROVE

Reviewed as a swap-and-key pass, not a re-derivation of wave 1. I spent the
budget on the three things this kind of pass gets wrong — a call site that now
calls something subtly different, a deleted helper with a live caller, a key
that breaks a comparison or a layout — and proved the first one by rendering
both trees rather than by reading.

Headline: rendered output is unchanged, and I mean byte-identical. I loaded
main's panel.luau and this branch's side by side in one luau state, gave
both the same stubs and a tr backed by the real en.json values, drove 13
states through the IPC surface, serialised each rendered tree with sorted props
and diffed:

$ luau parity.luau
base frames bytes: 73080
head frames bytes: 73080
TREES IDENTICAL across all 13 states

The 13 cover every swapped call site: fresh open (panel.no_day vs "--"),
day index 1 (key branch) and index 3 (verbatim Sat 29/08), a day with an
untagged row (tagLabel + Query.tagColor + Query.window with a predicate),
a day with nothing tracked, all-time + chip (Query.window with nil), and
the pager at page 1 / middle / last on both a 3-page and a 5-page list so the
strip has to clamp left, centre, and clamp right. That harness stubs an active
interval throughout, so a second run with timew.active false covers the branch
it misses:

$ luau idle.luau
idle clock rendered by both: true / true      # text=--:--
no_day rendered by both    : true / true      # text=--
IDLE TREES IDENTICAL

So Fmt.clock(os.time() - startEpoch) and formatElapsed(elapsed()) produce
the same label, Query.pageStrip slides the same way firstShown/lastShown
did, and Query.window returns the same shown slice and the same matching
count as the loop. Three findings, none of them in that arithmetic.

Findings

1 — P2. The untagged row's tooltip and the search box no longer agree with what the row says

timew/panel.luau:1028-1036

The sentinel split is right where you aimed it: filterFromRow (:799) compares
UNTAGGED, Query.tagColor (:1022) hashes UNTAGGED, and my locale probe
confirms the row dot keeps the same fill in a Spanish locale. The gap is the
other consumer of that string. passesFilter (:908) matches the needle against
row.tag, which is still (untagged) — but the row now reads
tr("panel.untagged"). So the string on screen is no longer the string that
matches:

day scope, no filter          : (sin etiqueta),deep work
search 'sin etiqueta' (shown) :                              <- zero rows
search 'untagged' (invisible) : (sin etiqueta)

A user who reads the row and types it into the search box gets nothing; only
the English literal, which appears nowhere on their screen, works. Before this
PR the row read (untagged) in every locale and searching it worked. The
tooltip you touched on :1036 makes it worse by promising the action in the
user's own language — on a row whose click filterFromRow deliberately
refuses.

Latent today: only en.json ships, so tr("panel.untagged") returns the
sentinel and nothing is reachable. That is why this is not a block.

Making the search agree would mean matching the label in passesFilter and
in Timew.parseIntervals, since all-time filters upstream — two call sites and
a tr in a per-row loop, for a row you have already decided is not a filter
target. The cheaper fix is to stop advertising the action, which also drops a
tagLabel call:

					contentAlign = "start",
					selected = filter ~= "" and Timew.tagMatches(row.tag, filter),
					tooltip = if row.tag == UNTAGGED then nil else noctalia.tr("panel.filter_by", { tag = row.tag }),
					onClick = "onRow" .. i,

An absent tooltip prop is already how the page-number buttons at :1245 are
built, so this needs no new host behaviour.

2 — P3. --:-- and -- became keys under the rule that kept ‹/› literal

timew/panel.luau:1069-1072, timew/translations/en.json

The comment you added at :1227 states the rule and states it well: a glyph
placeholder stays a literal because "a word here pushes the number strip off a
row pinned to RIGHT_W". panel.clock_idle sits in a tighter box than the
pager does. Its label is fontSize = 28, fontWeight = "bold" inside
ui.column({ gap = 4, width = LEFT_W - ACTION_W - 24 }) — 120 px (:1100) — and
the panel cannot clip. --:-- is five narrow glyphs; a word is not. en.json
already carries an unused panel.idle: "Not tracking", which is exactly the
prose a translator handed panel.clock_idle would reach for. panel.no_day
has the same shape on the scope button in the width = RIGHT_W header.

Both are placeholders for a value, not prose, by the test the same patch
applies to ‹/›. Either keep them literal or record the ceiling where a
translator will see it — a "_comment" key beside them in en.json naming the
120 px and the 28 pt costs nothing and is the only place the constraint could
land, since translators never read panel.luau.

The ruling you asked for on ‹/›: correct as literals, and for the reason
given. They are direction, not language; RTL is the one case that would want
them mirrored, and the shell has no RTL story at this API to mirror them into.

3 — P3. bar.luau:156 is now the only screen the sentinel still reaches

timew/bar.luau:154-157

setTooltip(tags) renders the value :180 set to "(untagged)", verbatim.
bar.luau contains zero noctalia.tr calls, so it has no route to
panel.untagged and this PR correctly left it alone. But the divergence is
new: before, both surfaces said (untagged); now the panel translates and the
bar does not, and your own screenshot item 10 puts them side by side. Worth an
issue rather than a change here — translating the bar means giving bar.luau
its first key, which is a separate decision from this one.

Verified

Gate, run here, not quoted from the description:

$ nix shell nixpkgs#luau -c luau test.luau
ok
$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings
$ git diff --stat main...HEAD --name-only
test.luau
timew/panel.luau
timew/translations/en.json
$ grep -c 'getfenv\|setfenv' timew/panel.luau timew/bar.luau
timew/panel.luau:0
timew/bar.luau:0

Three files, no excluded file, no getfenv in either plugin entry (test.luau
keeps two, which is where it is allowed).

The panel.untagged locale experiment. I ran the one you would expect
first: set panel.untagged to ZZ_LOCALE_PROBE_ZZ in en.json, ran the gate.
Both stages still pass — because test.luau never reads en.json (no
reference to it anywhere in the file; the harness stubs tr to hand back the
key). So the suite alone proves the split is wired, not that it is
locale-independent. Restored: cmp byte-identical, md5 fbfd4772… before and
after, git status --porcelain empty.

The experiment that does prove it runs panel.luau twice with two real
translation tables, English and a Spanish one that also flips the chip glyph to
the other side:

EN rows                 : (untagged),deep work
ES rows                 : (sin etiqueta),deep work
EN row dot fills        : onRow1=primary,onRow2=secondary
ES row dot fills        : onRow1=primary,onRow2=secondary     <- identical
EN chip after row1/row2 : false / true
ES chip after row1/row2 : false / true                        <- identical
EN chip text            : deep work  ✕
ES chip text            : ✕  deep work
LOCALE PROBE: ok -- display translates, filtering does not

Clicking the untagged row sets no filter in either locale; clicking the real
tag sets one in both; the row dot's colour role does not move; the sentinel
never appears on the Spanish tree and the Spanish string never appears on the
English one. A locale changes what the row says and nothing about which rows
are filterable — which is finding 1's mirror image: it is too independent in
the search box.

Deleted helpers. Reproduced your grep and widened it to catch a survivor in
a comment or a string:

$ grep -nE '(^|[^.[:alnum:]_])(trim|tagColor|formatElapsed|formatSpan|elapsed)\(' timew/panel.luau
(none)
$ grep -nE '\b(formatElapsed|formatSpan|tagColor|elapsed)\b' timew/panel.luau
4,56,1061,1308   prose ("elapsed time", the deletion note, the tick comment)
214,249,1022     Query.tagColor

Every survivor is either prose or the qualified Query.tagColor. The export
table returns Fmt, Parse, Query, Timew, all four defined. test.luau
reaches for panel.Fmt/Parse/Query/Timew and bar.Config/formatElapsed only —
the single panel.formatElapsed left in it is inside the comment at :33, not a
call.

Keys. Every noctalia.tr("…") in timew/*.luau resolves against
en.json: 22 used, 0 missing. panel.no_day/today_short/yesterday reach it
through Fmt.dayLabel's return rather than a literal call site, so they are
live too. Byte-level check on the four moved literals, since a lost space is
the classic key regression:

main  1153: text = filter .. "  M-bM-^\M-^U"      # two spaces, U+2715
en.json     {tag}  M-bM-^\M-^U                    # two spaces, U+2715
main  1284: `{listOffset + 1}-{listOffset + #shown} / {matching}`
en.json     {first}-{last} / {total}
main  1080/1091: "--" / "--:--"   ==   en.json  -- / --:--

The chip gap and the glyph survive substitution unchanged, and the tree diff
above confirms it end to end. Numeric substitution into page_range is not new
ground — panel.trend has passed { days = DAYS } since before this PR.

Header overflow. panel.filter_chip costs nothing in English: the chip is
the same string it was, and the header row still contains two scope buttons, the
chip, a flexGrow spacer and a DUR_W total inside width = RIGHT_W. The
variable part is the user's tag, which was already unbounded before this PR, so
the key adds no new overflow the panel did not already have.

Asserts. No pre-existing assert weakened. The two loops that compared a
deleted duplicate against its replacement lost the deleted operand and kept
every value and every message: Fmt.span still runs the same nine, the
formatElapsed/Fmt.clock drift loop still runs the same fourteen including
the fractional and negative ones. The one changed assert
(Fmt.dayLabel(nil, 1)) tracks the intended behaviour change. Net five new
blocks.

listOffset past the match count. You asked, so: Query.window returns an
empty shown and a true matching, current lands above pages, no page
button reads selected, ‹ stays enabled and › disabled, and the range label
reads backwards (17-16 / 10). That is reachable — fetchDays → loadDay(sel, true) does not rewind the pager and the day can shrink under it — but it is
byte-identical to main's arithmetic, so it is not this PR's. Flagging it only
so it is written down somewhere.

Answers

elapsed deleted rather than moved — agreed, and it is the best of the three
calls in this PR. The guard was dead: the only call site sat in the else of
if not active, so elapsed() could only ever return os.time() - startEpoch.
Moving a dead branch into the pure core would have meant testing it, and a test
for an unreachable branch is worse than no test. Fmt.clock clamps negatives
with math.max(0, …), which is the only thing the guard was protecting against
in practice, and my idle-state parity run exercises both sides of the branch it
used to sit in. The startEpoch = 0 while active case behaves identically
before and after, so nothing regressed there either.

shQuote stays a module local — agreed, and the reasoning holds up under
grep. Three callers, exactly as claimed: Timew.tagArg (:398),
Timew.cmd.intervalsForDay (:447), startTag (:728). It is not text → data,
so Parse is the wrong table for it; and since Timew.tagArg is the exported
injection surface, moving shQuote into Parse would give test.luau a second
place to point quoting asserts at without giving it anything new to assert. The
startTag third caller is the same no require between entries consequence
Timew.exec already documents, and the comment says so instead of re-arguing
it.

Scope beyond the brief — right call, and the diff is the argument. Four
tested functions with zero production callers while the panel carried its own
copy of each is not "extra scope", it is the wave being finished. Shipping the
pure core as dead code would have left the next reader to discover the panel
disagreed with its own core. And the extra scope is subtraction: panel.luau
is 88 lines shorter for it, with the growth in comments rather than code. The
one thing I would push back on is the stated reason for passesFilter (:907)
being a module-level function — one closure per second is not a 25 ms budget
problem. The code is right anyway; a named predicate beside the call reads
better than an inline one, which is reason enough on its own.

Panel states worth screenshotting after merge

Your list is the right list; the tree diff removes the need for most of it.
These are the ones a tree cannot answer, in the order I would shoot them:

  1. Tracking a multi-word tag, panel and bar side by side — the clocks must
    show the same second, and the card must not resize when tracking starts.
    elapsed() was inlined and SUBJECT_H is doing the work; both are geometry.
  2. A day whose intervals include an untagged one — the row's dot colour
    against a pre-merge shot. The tree proves the role name; only pixels prove
    the theme resolves it the same way.
  3. Filtered, All time, a tag on some days — bars at primary/0.5, selected
    at full primary, day letters under lit bars in primary. The one place
    Query.barRoles could have moved a role.
  4. A tag with 40+ matches, pages 1 / 3 / 5 — the strip must slide, not grow,
    and the range label must not wrap off RIGHT_W.
  5. The chip — <tag> ✕ with both spaces, hover for Clear tag filter,
    click to clear.
  6. Idle before the first fetch — clock --:--, scope button --. If either
    renders as its key name the merge is missing an en.json.
  7. A day with nothing tracked under a dated scope button — Sat 29/08 plus
    Nothing tracked, the two states finding 2 is about.
**Verdict: APPROVE** Reviewed as a swap-and-key pass, not a re-derivation of wave 1. I spent the budget on the three things this kind of pass gets wrong — a call site that now calls something subtly different, a deleted helper with a live caller, a key that breaks a comparison or a layout — and proved the first one by rendering both trees rather than by reading. **Headline: rendered output is unchanged, and I mean byte-identical.** I loaded `main`'s `panel.luau` and this branch's side by side in one luau state, gave both the same stubs and a `tr` backed by the real `en.json` values, drove 13 states through the IPC surface, serialised each rendered tree with sorted props and diffed: ``` $ luau parity.luau base frames bytes: 73080 head frames bytes: 73080 TREES IDENTICAL across all 13 states ``` The 13 cover every swapped call site: fresh open (`panel.no_day` vs `"--"`), day index 1 (key branch) and index 3 (verbatim `Sat 29/08`), a day with an untagged row (`tagLabel` + `Query.tagColor` + `Query.window` with a predicate), a day with nothing tracked, all-time + chip (`Query.window` with `nil`), and the pager at page 1 / middle / last on both a 3-page and a 5-page list so the strip has to clamp left, centre, and clamp right. That harness stubs an active interval throughout, so a second run with `timew.active` false covers the branch it misses: ``` $ luau idle.luau idle clock rendered by both: true / true # text=--:-- no_day rendered by both : true / true # text=-- IDLE TREES IDENTICAL ``` So `Fmt.clock(os.time() - startEpoch)` and `formatElapsed(elapsed())` produce the same label, `Query.pageStrip` slides the same way `firstShown`/`lastShown` did, and `Query.window` returns the same `shown` slice and the same `matching` count as the loop. Three findings, none of them in that arithmetic. ## Findings ### 1 — P2. The untagged row's tooltip and the search box no longer agree with what the row says `timew/panel.luau:1028-1036` The sentinel split is right where you aimed it: `filterFromRow` (:799) compares `UNTAGGED`, `Query.tagColor` (:1022) hashes `UNTAGGED`, and my locale probe confirms the row dot keeps the same fill in a Spanish locale. The gap is the other consumer of that string. `passesFilter` (:908) matches the needle against `row.tag`, which is still `(untagged)` — but the row now reads `tr("panel.untagged")`. So the string on screen is no longer the string that matches: ``` day scope, no filter : (sin etiqueta),deep work search 'sin etiqueta' (shown) : <- zero rows search 'untagged' (invisible) : (sin etiqueta) ``` A user who reads the row and types it into the search box gets nothing; only the English literal, which appears nowhere on their screen, works. Before this PR the row read `(untagged)` in every locale and searching it worked. The tooltip you touched on :1036 makes it worse by promising the action in the user's own language — on a row whose click `filterFromRow` deliberately refuses. Latent today: only `en.json` ships, so `tr("panel.untagged")` returns the sentinel and nothing is reachable. That is why this is not a block. Making the search agree would mean matching the label in `passesFilter` *and* in `Timew.parseIntervals`, since all-time filters upstream — two call sites and a `tr` in a per-row loop, for a row you have already decided is not a filter target. The cheaper fix is to stop advertising the action, which also drops a `tagLabel` call: ```suggestion contentAlign = "start", selected = filter ~= "" and Timew.tagMatches(row.tag, filter), tooltip = if row.tag == UNTAGGED then nil else noctalia.tr("panel.filter_by", { tag = row.tag }), onClick = "onRow" .. i, ``` An absent `tooltip` prop is already how the page-number buttons at :1245 are built, so this needs no new host behaviour. ### 2 — P3. `--:--` and `--` became keys under the rule that kept `‹`/`›` literal `timew/panel.luau:1069-1072`, `timew/translations/en.json` The comment you added at :1227 states the rule and states it well: a glyph placeholder stays a literal because "a word here pushes the number strip off a row pinned to `RIGHT_W`". `panel.clock_idle` sits in a tighter box than the pager does. Its label is `fontSize = 28, fontWeight = "bold"` inside `ui.column({ gap = 4, width = LEFT_W - ACTION_W - 24 })` — 120 px (:1100) — and the panel cannot clip. `--:--` is five narrow glyphs; a word is not. `en.json` already carries an unused `panel.idle: "Not tracking"`, which is exactly the prose a translator handed `panel.clock_idle` would reach for. `panel.no_day` has the same shape on the scope button in the `width = RIGHT_W` header. Both are placeholders for a value, not prose, by the test the same patch applies to `‹`/`›`. Either keep them literal or record the ceiling where a translator will see it — a `"_comment"` key beside them in `en.json` naming the 120 px and the 28 pt costs nothing and is the only place the constraint could land, since translators never read `panel.luau`. The ruling you asked for on `‹`/`›`: correct as literals, and for the reason given. They are direction, not language; RTL is the one case that would want them mirrored, and the shell has no RTL story at this API to mirror them into. ### 3 — P3. `bar.luau:156` is now the only screen the sentinel still reaches `timew/bar.luau:154-157` `setTooltip(tags)` renders the value :180 set to `"(untagged)"`, verbatim. `bar.luau` contains zero `noctalia.tr` calls, so it has no route to `panel.untagged` and this PR correctly left it alone. But the divergence is new: before, both surfaces said `(untagged)`; now the panel translates and the bar does not, and your own screenshot item 10 puts them side by side. Worth an issue rather than a change here — translating the bar means giving `bar.luau` its first key, which is a separate decision from this one. ## Verified **Gate**, run here, not quoted from the description: ``` $ nix shell nixpkgs#luau -c luau test.luau ok $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings $ git diff --stat main...HEAD --name-only test.luau timew/panel.luau timew/translations/en.json $ grep -c 'getfenv\|setfenv' timew/panel.luau timew/bar.luau timew/panel.luau:0 timew/bar.luau:0 ``` Three files, no excluded file, no `getfenv` in either plugin entry (`test.luau` keeps two, which is where it is allowed). **The `panel.untagged` locale experiment.** I ran the one you would expect first: set `panel.untagged` to `ZZ_LOCALE_PROBE_ZZ` in `en.json`, ran the gate. Both stages still pass — because `test.luau` never reads `en.json` (no reference to it anywhere in the file; the harness stubs `tr` to hand back the key). So the suite alone proves the split is *wired*, not that it is *locale-independent*. Restored: `cmp` byte-identical, md5 `fbfd4772…` before and after, `git status --porcelain` empty. The experiment that does prove it runs `panel.luau` twice with two real translation tables, English and a Spanish one that also flips the chip glyph to the other side: ``` EN rows : (untagged),deep work ES rows : (sin etiqueta),deep work EN row dot fills : onRow1=primary,onRow2=secondary ES row dot fills : onRow1=primary,onRow2=secondary <- identical EN chip after row1/row2 : false / true ES chip after row1/row2 : false / true <- identical EN chip text : deep work ✕ ES chip text : ✕ deep work LOCALE PROBE: ok -- display translates, filtering does not ``` Clicking the untagged row sets no filter in either locale; clicking the real tag sets one in both; the row dot's colour role does not move; the sentinel never appears on the Spanish tree and the Spanish string never appears on the English one. A locale changes what the row says and nothing about which rows are filterable — which is finding 1's mirror image: it is *too* independent in the search box. **Deleted helpers.** Reproduced your grep and widened it to catch a survivor in a comment or a string: ``` $ grep -nE '(^|[^.[:alnum:]_])(trim|tagColor|formatElapsed|formatSpan|elapsed)\(' timew/panel.luau (none) $ grep -nE '\b(formatElapsed|formatSpan|tagColor|elapsed)\b' timew/panel.luau 4,56,1061,1308 prose ("elapsed time", the deletion note, the tick comment) 214,249,1022 Query.tagColor ``` Every survivor is either prose or the qualified `Query.tagColor`. The export table returns `Fmt`, `Parse`, `Query`, `Timew`, all four defined. `test.luau` reaches for `panel.Fmt/Parse/Query/Timew` and `bar.Config/formatElapsed` only — the single `panel.formatElapsed` left in it is inside the comment at :33, not a call. **Keys.** Every `noctalia.tr("…")` in `timew/*.luau` resolves against `en.json`: 22 used, 0 missing. `panel.no_day`/`today_short`/`yesterday` reach it through `Fmt.dayLabel`'s return rather than a literal call site, so they are live too. Byte-level check on the four moved literals, since a lost space is the classic key regression: ``` main 1153: text = filter .. " M-bM-^\M-^U" # two spaces, U+2715 en.json {tag} M-bM-^\M-^U # two spaces, U+2715 main 1284: `{listOffset + 1}-{listOffset + #shown} / {matching}` en.json {first}-{last} / {total} main 1080/1091: "--" / "--:--" == en.json -- / --:-- ``` The chip gap and the glyph survive substitution unchanged, and the tree diff above confirms it end to end. Numeric substitution into `page_range` is not new ground — `panel.trend` has passed `{ days = DAYS }` since before this PR. **Header overflow.** `panel.filter_chip` costs nothing in English: the chip is the same string it was, and the header row still contains two scope buttons, the chip, a `flexGrow` spacer and a `DUR_W` total inside `width = RIGHT_W`. The variable part is the user's tag, which was already unbounded before this PR, so the key adds no new overflow the panel did not already have. **Asserts.** No pre-existing assert weakened. The two loops that compared a deleted duplicate against its replacement lost the deleted operand and kept every value and every message: `Fmt.span` still runs the same nine, the `formatElapsed`/`Fmt.clock` drift loop still runs the same fourteen including the fractional and negative ones. The one changed assert (`Fmt.dayLabel(nil, 1)`) tracks the intended behaviour change. Net five new blocks. **`listOffset` past the match count.** You asked, so: `Query.window` returns an empty `shown` and a true `matching`, `current` lands above `pages`, no page button reads selected, `‹` stays enabled and `›` disabled, and the range label reads backwards (`17-16 / 10`). That is reachable — `fetchDays` → `loadDay(sel, true)` does not rewind the pager and the day can shrink under it — but it is byte-identical to `main`'s arithmetic, so it is not this PR's. Flagging it only so it is written down somewhere. ## Answers **`elapsed` deleted rather than moved — agreed, and it is the best of the three calls in this PR.** The guard was dead: the only call site sat in the `else` of `if not active`, so `elapsed()` could only ever return `os.time() - startEpoch`. Moving a dead branch into the pure core would have meant testing it, and a test for an unreachable branch is worse than no test. `Fmt.clock` clamps negatives with `math.max(0, …)`, which is the only thing the guard was protecting against in practice, and my idle-state parity run exercises both sides of the branch it used to sit in. The `startEpoch = 0` while `active` case behaves identically before and after, so nothing regressed there either. **`shQuote` stays a module local — agreed, and the reasoning holds up under grep.** Three callers, exactly as claimed: `Timew.tagArg` (:398), `Timew.cmd.intervalsForDay` (:447), `startTag` (:728). It is not text → data, so `Parse` is the wrong table for it; and since `Timew.tagArg` is the exported injection surface, moving `shQuote` into `Parse` would give `test.luau` a second place to point quoting asserts at without giving it anything new to assert. The `startTag` third caller is the same `no require between entries` consequence `Timew.exec` already documents, and the comment says so instead of re-arguing it. **Scope beyond the brief — right call, and the diff is the argument.** Four tested functions with zero production callers while the panel carried its own copy of each is not "extra scope", it is the wave being finished. Shipping the pure core as dead code would have left the next reader to discover the panel disagreed with its own core. And the extra scope is subtraction: `panel.luau` is 88 lines shorter for it, with the growth in comments rather than code. The one thing I would push back on is the stated reason for `passesFilter` (:907) being a module-level function — one closure per second is not a 25 ms budget problem. The code is right anyway; a named predicate beside the call reads better than an inline one, which is reason enough on its own. ## Panel states worth screenshotting after merge Your list is the right list; the tree diff removes the need for most of it. These are the ones a tree cannot answer, in the order I would shoot them: 1. **Tracking a multi-word tag, panel and bar side by side** — the clocks must show the same second, and the card must not resize when tracking starts. `elapsed()` was inlined and `SUBJECT_H` is doing the work; both are geometry. 2. **A day whose intervals include an untagged one** — the row's dot colour against a pre-merge shot. The tree proves the role name; only pixels prove the theme resolves it the same way. 3. **Filtered, All time, a tag on some days** — bars at `primary/0.5`, selected at full `primary`, day letters under lit bars in `primary`. The one place `Query.barRoles` could have moved a role. 4. **A tag with 40+ matches, pages 1 / 3 / 5** — the strip must slide, not grow, and the range label must not wrap off `RIGHT_W`. 5. **The chip** — `<tag> ✕` with both spaces, hover for `Clear tag filter`, click to clear. 6. **Idle before the first fetch** — clock `--:--`, scope button `--`. If either renders as its key name the merge is missing an `en.json`. 7. **A day with nothing tracked under a dated scope button** — `Sat 29/08` plus `Nothing tracked`, the two states finding 2 is about. 
The untagged row's tooltip offered "Filter by (untagged)" through tr, so in a
translated locale it promised the action in the user's own language on the one
row filterFromRow deliberately refuses. Search cannot reach it either:
passesFilter matches the needle against the UNTAGGED sentinel, not against the
translated label the row displays. Drop the tooltip on that row rather than
teach passesFilter and Timew.parseIntervals to match a per-row tr call for a row
that is not a filter target. An absent tooltip prop is how the page-number
buttons are already built.

Also revert panel.clock_idle and panel.no_day to literals, applying the rule the
same pass wrote for the pager chevrons: a placeholder is the shape of the value,
not text about it, so a key buys nothing a translator could get right and costs
the one thing they could get wrong. The idle clock is 28 pt bold in a 120 px
column and the panel cannot clip; en.json's unused panel.idle ("Not tracking")
is exactly the prose a key there would invite.

Asserts pin all three: the untagged row is displayed translated, is never a
filter target, and carries no tooltip while the row beside it does.
yuris commented 2026-08-31 19:17:39 +00:00
Author
Owner
Copy link

All three findings handled in 7cf16da. Thanks for rendering both trees rather
than reading them — the parity dump is what made finding 1 a one-line fix
instead of an argument.

1 — P2, applied as suggested

tooltip = if row.tag == UNTAGGED then nil else noctalia.tr("panel.filter_by", { tag = row.tag }).
Your analysis of the alternative is right and I did not attempt it: teaching
passesFilter and Timew.parseIntervals to match the translated label means a
tr call per row in a per-second loop, to make searchable a row whose click
filterFromRow refuses on purpose. The row is not a filter target by any route,
so it should not advertise one. The else branch drops back to row.tag — the
sentinel is the only tag tagLabel rewrites, so the surviving branch never
needed it.

Re-ran your comparison against the untagged state, base = this PR as you
reviewed it, head = the tooltip fix alone, same stubs, props sorted:

$ luau parity.luau
base bytes: 2702
head bytes: 2658
--- base 32:  button(… onClick=onRow1 selected=false text=panel.untagged tooltip=panel.filter_by{tag=panel.untagged} variant=ghost)
+++ head 32:  button(… onClick=onRow1 selected=false text=panel.untagged variant=ghost)
differing nodes: 1 of 40

head row: button(… onClick=onRow1 selected=false text=panel.untagged variant=ghost)
head row: button(… onClick=onRow2 selected=false text=deep work tooltip=panel.filter_by{tag=deep work} variant=ghost)

One node moves, the prop leaves rather than emptying, and the real tag's row
beside it keeps its tooltip.

2 — P3, resolved by reverting both to literals

I took the first of your two options, because the rule I wrote at :1227
already covers them and I would rather have one rule than one rule plus a
footnote:

‹ and › stay literals: they are the button's shape, not its text, so a key
buys nothing a translator could get right and costs the one thing they could
get wrong.

--:-- and -- are the shape of the value, not text about it, by exactly that
test. The _comment route records the ceiling in the one place a translator
might look, but it only documents the failure — a translation memory or an
MT pass never reads it, and panel.idle: "Not tracking" sitting three lines
above panel.clock_idle is precisely the substitution it would make. A literal
removes the failure instead. The clock is the acute case at fontSize = 28
bold in a 120 px column; panel.no_day shares the RIGHT_W header row with the
second scope button and the chip. Neither can clip, and neither gains anything
from a locale.

Fmt.dayLabel(nil, …) returns "--", false again, both keys are out of
en.json, and the comments at both sites and in test.luau now cite the pager
rule by name so the next reader finds one rule applied three times. I left the
unused panel.idle alone: it predates this PR and translation keys are public
surface.

3 — P3, recorded, not changed

Agreed it is out of scope: translating bar.luau:156 means giving that entry
its first noctalia.tr call, which is a decision about the bar, not about this
swap. Recorded in REFACTOR.md under ## Deferred, together with the
listOffset-past-matching case from your Verified section — that one is
byte-identical to main's arithmetic, so it is older than this wave, but it
deserved to be written down somewhere and now it is.

Asserts

The untagged row's contract is pinned in all three parts, read off the tree
before the clicks that change the list:

local tips = table.concat(clickedProp(panelTree, "^onRow%d+$", "tooltip"), ",")
assert(tips == "nil,panel.filter_by", …)

clickedTexts is now clickedProp(…, "text"); the walk keeps its text type
guard whichever prop it reads, so both lists index the same nodes, and
tostring keeps an absent prop in the list as "nil" rather than dropping the
node and sliding the rest left. The control row is what stops it passing if
every tooltip disappears. Two more assert pairs cover finding 2: the scope
button shows -- and not panel.no_day, the idle clock shows --:-- and not
panel.clock_idle.

Liveness by mutation, each reverted byte-identically (cmp clean, panel.luau
md5 7cd2c2df… before and after):

tooltip restored on the untagged row
  test.luau:665: the untagged row carries no tooltip, the real tag's row does: panel.filter_by,panel.filter_by
Fmt.dayLabel(nil) back to a key
  test.luau:66: the empty state is a literal placeholder, not a key a translator could widen
idle clock back to a key
  test.luau:590: idle before the first fetch, the clock shows the --:-- placeholder

Full-head parity on the same untagged state now shows three differing nodes of
40 — the tooltip plus the two reverted literals — and nothing else:

--- base  6:  label(color=on_surface_variant fontSize=28 fontWeight=bold text=panel.clock_idle)
+++ head  6:  label(color=on_surface_variant fontSize=28 fontWeight=bold text=--:--)
--- base 24:  button(… onClick=onScopeDay selected=false text=panel.no_day variant=ghost)
+++ head 24:  button(… onClick=onScopeDay selected=false text=-- variant=ghost)
--- base 32:  button(… onRow1 … tooltip=panel.filter_by{tag=panel.untagged} …)
+++ head 32:  button(… onRow1 … variant=ghost)

Gate

$ nix shell nixpkgs#luau -c luau test.luau
ok
$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings
$ git diff --stat main...HEAD --name-only
test.luau
timew/panel.luau
timew/translations/en.json
$ git status --porcelain

Keys: 20 literal noctalia.tr("…") call sites in timew/*.luau, 0 missing from
en.json; the only non-literal tr argument left is Fmt.dayLabel's return,
which the isKey guard now keeps -- out of.

All three findings handled in `7cf16da`. Thanks for rendering both trees rather than reading them — the parity dump is what made finding 1 a one-line fix instead of an argument. ## 1 — P2, applied as suggested `tooltip = if row.tag == UNTAGGED then nil else noctalia.tr("panel.filter_by", { tag = row.tag })`. Your analysis of the alternative is right and I did not attempt it: teaching `passesFilter` and `Timew.parseIntervals` to match the translated label means a `tr` call per row in a per-second loop, to make searchable a row whose click `filterFromRow` refuses on purpose. The row is not a filter target by any route, so it should not advertise one. The `else` branch drops back to `row.tag` — the sentinel is the only tag `tagLabel` rewrites, so the surviving branch never needed it. Re-ran your comparison against the untagged state, base = this PR as you reviewed it, head = the tooltip fix alone, same stubs, props sorted: ``` $ luau parity.luau base bytes: 2702 head bytes: 2658 --- base 32: button(… onClick=onRow1 selected=false text=panel.untagged tooltip=panel.filter_by{tag=panel.untagged} variant=ghost) +++ head 32: button(… onClick=onRow1 selected=false text=panel.untagged variant=ghost) differing nodes: 1 of 40 head row: button(… onClick=onRow1 selected=false text=panel.untagged variant=ghost) head row: button(… onClick=onRow2 selected=false text=deep work tooltip=panel.filter_by{tag=deep work} variant=ghost) ``` One node moves, the prop leaves rather than emptying, and the real tag's row beside it keeps its tooltip. ## 2 — P3, resolved by reverting both to literals I took the first of your two options, because the rule I wrote at `:1227` already covers them and I would rather have one rule than one rule plus a footnote: > ‹ and › stay literals: they are the button's shape, not its text, so a key > buys nothing a translator could get right and costs the one thing they could > get wrong. `--:--` and `--` are the shape of the value, not text about it, by exactly that test. The `_comment` route records the ceiling in the one place a translator might look, but it only *documents* the failure — a translation memory or an MT pass never reads it, and `panel.idle: "Not tracking"` sitting three lines above `panel.clock_idle` is precisely the substitution it would make. A literal removes the failure instead. The clock is the acute case at `fontSize = 28` bold in a 120 px column; `panel.no_day` shares the `RIGHT_W` header row with the second scope button and the chip. Neither can clip, and neither gains anything from a locale. `Fmt.dayLabel(nil, …)` returns `"--", false` again, both keys are out of `en.json`, and the comments at both sites and in `test.luau` now cite the pager rule by name so the next reader finds one rule applied three times. I left the unused `panel.idle` alone: it predates this PR and translation keys are public surface. ## 3 — P3, recorded, not changed Agreed it is out of scope: translating `bar.luau:156` means giving that entry its first `noctalia.tr` call, which is a decision about the bar, not about this swap. Recorded in `REFACTOR.md` under `## Deferred`, together with the `listOffset`-past-`matching` case from your Verified section — that one is byte-identical to `main`'s arithmetic, so it is older than this wave, but it deserved to be written down somewhere and now it is. ## Asserts The untagged row's contract is pinned in all three parts, read off the tree before the clicks that change the list: ```lua local tips = table.concat(clickedProp(panelTree, "^onRow%d+$", "tooltip"), ",") assert(tips == "nil,panel.filter_by", …) ``` `clickedTexts` is now `clickedProp(…, "text")`; the walk keeps its `text` type guard whichever prop it reads, so both lists index the same nodes, and `tostring` keeps an absent prop in the list as `"nil"` rather than dropping the node and sliding the rest left. The control row is what stops it passing if every tooltip disappears. Two more assert pairs cover finding 2: the scope button shows `--` and not `panel.no_day`, the idle clock shows `--:--` and not `panel.clock_idle`. Liveness by mutation, each reverted byte-identically (`cmp` clean, `panel.luau` md5 `7cd2c2df…` before and after): ``` tooltip restored on the untagged row test.luau:665: the untagged row carries no tooltip, the real tag's row does: panel.filter_by,panel.filter_by Fmt.dayLabel(nil) back to a key test.luau:66: the empty state is a literal placeholder, not a key a translator could widen idle clock back to a key test.luau:590: idle before the first fetch, the clock shows the --:-- placeholder ``` Full-head parity on the same untagged state now shows three differing nodes of 40 — the tooltip plus the two reverted literals — and nothing else: ``` --- base 6: label(color=on_surface_variant fontSize=28 fontWeight=bold text=panel.clock_idle) +++ head 6: label(color=on_surface_variant fontSize=28 fontWeight=bold text=--:--) --- base 24: button(… onClick=onScopeDay selected=false text=panel.no_day variant=ghost) +++ head 24: button(… onClick=onScopeDay selected=false text=-- variant=ghost) --- base 32: button(… onRow1 … tooltip=panel.filter_by{tag=panel.untagged} …) +++ head 32: button(… onRow1 … variant=ghost) ``` ## Gate ``` $ nix shell nixpkgs#luau -c luau test.luau ok $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings $ git diff --stat main...HEAD --name-only test.luau timew/panel.luau timew/translations/en.json $ git status --porcelain ``` Keys: 20 literal `noctalia.tr("…")` call sites in `timew/*.luau`, 0 missing from `en.json`; the only non-literal `tr` argument left is `Fmt.dayLabel`'s return, which the `isKey` guard now keeps `--` out of.
yuris merged commit 6617450d82 into main 2026-08-31 19:17:44 +00:00
yuris deleted branch refactor/t4-swap 2026-08-31 19:17:44 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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!7
No description provided.