refactor(timew): split panel.luau into modules, teach the suite to load them #14

Merged

yuris merged 1 commit from upgrade/u4-modules into main 2026-09-01 18:10:13 +00:00

Conversation 1 Commits 1 Files changed 13 +1170 -875

yuris commented 2026-09-01 16:24:53 +00:00

Owner

Copy link

panel.luau was 1701 lines holding six layers. It is 1025 now and holds only what an
entry has to: state, the handlers that move it, the region builders, and the assembly of
the tree.

The graph

graph TD
  fmt[fmt.luau · 61 · pure]
  parse[parse.luau · 98 · pure]
  query[query.luau · 76 · pure]
  timew[timew.luau · 376]
  vm[viewmodel.luau · 172]
  panel[panel.luau · 1025 · entry]
  bar[bar.luau · 242 · entry]

  parse --> fmt
  timew --> fmt
  timew --> parse
  vm --> fmt
  vm --> parse
  vm --> query
  vm --> timew
  panel --> fmt
  panel --> parse
  panel --> query
  panel --> timew
  panel --> vm
  bar --> fmt
  bar --> parse

Acyclicity, how it was checked. The seven files' require("./x.luau") strings were
extracted and walked with a colouring DFS. Result: acyclic, topological order
fmt < parse < query < timew < viewmodel < panel < bar; every require target has exactly
one alias file and no alias file lacks a requirer. No back-edge was wanted and none was
added — state travels down as an argument, never up as a require.

Note that the runtime gate for this is the shell's, not the suite's: pushRequiredModule
walks m_moduleStack and raises circular require: <full chain>
(luau_host.cpp:2976-2985). The bare luau CLI does not detect cycles — a two-file
cycle built in /tmp ran clean and exited 0 under it — so a cycle would pass
luau test.luau and fail to load the plugin. That asymmetry is why the split keeps the
graph shallow rather than routing edges through an aggregator.

Modules

file	lines	contents	reads a host global?
fmt.luau	61	clock, span, dayLabel	no
parse.luau	98	UNTAGGED, trim, toast, unquote, duration, clock, interval	no
query.luau	76	matching, barRoles, tagColor, barPeak	no
timew.luau	376	DAYS/DAY_WALK_MAX/ALL_WALK_MAX, the parsers, cmd.*, the runAsync wrappers, exec	noctalia, inside function bodies only
viewmodel.luau	172	build(state), TAG_COLORS	os.time
panel.luau	1025	layout constants, state, apply/commit, fetches, handlers, builders, render, onOpen/update/onIpc	yes
bar.luau	242	Config, tick, sync, render, onClick	yes

Three deviations from the shape in UPGRADE.md:

No config.luau. The manifest declares settings on the bar entry only; the panel reads
none. Config has exactly one caller, duplicates nothing, and extracting it would add a
file and a second stubbed environment to the suite for no present reader. U6 gives the
panel formatting settings — that is its first real second consumer, and the right task to
move it. A module with one caller is a file to open, not a boundary.

viewmodel.luau requires timew.luau. It reads exactly two pure values from it:
tagMatches (the same substring test the walk filters with) and DAYS (the chart is as
wide as the fetch). It calls nothing impure. Moving tagMatches into query.luau would
cut the edge, but the tracker's own layering puts the view model above the commands, and
the move would churn the suite's T.tagMatches asserts and the pure-core counter's note
for an edge that is a read of two constants.

The three fetch bounds are exported. DAYS, DAY_WALK_MAX and ALL_WALK_MAX were
locals of the one file that both built the commands and drew the chart. They belong with
the commands, and the panel needs DAYS for RIGHT_W and the trend caption. So they live
in timew.luau and are read off it. The suite's frozen scripts stay frozen literals, and a
new assert ties the exported numbers to them — a pair that could not be checked against
each other at all before.

bar.luau: what is shared and what is not

The bar is its own entry with its own Luau VM and its own module cache, so a shared module
means each VM loads its own instance of one file — the sharing is of source, not of
state, and neither shared module has any state to share.

helper	ruling	why
formatElapsed	share → Fmt.clock	byte-identical body; the two surfaces must render one interval as one string
trim	share → Parse.trim	byte-identical, and toast needs it anyway
unquote	share → Parse.unquote	byte-identical
toast	share → Parse.toast	carries two user-visible strings that existed twice: "timew is not on PATH" and "timew failed (exit N)"
"(untagged)"	share → Parse.UNTAGGED	the strongest case in the file: a wire value the bar publishes into timew.tags and the panel compares with ~=. It was a literal in each entry
elapsed	keep	reads bar's own module state; there is nothing to share
Config	keep	see above
the sync shell program	keep, and pin	requiring ./timew.luau for one string would put a bar tick downstream of the whole command layer. It was an unguarded duplicate; it is now held against Timew.cmd.activeInterval(), the way the timew start argv already was
timew start / timew stop argv	keep	the bar's tag comes from its own Config and its stop is a branch of one click handler; already pinned against the panel's

The test loader, and why it is not the one that was specified

The brief called for a loader that resolves ./x.luau itself, caches per path, injects
stubbed globals and injects itself as require. Half of that is impossible on this
runtime, and it was checked rather than assumed:

• No io. type(io) is nil in luau. There is no way to read a module's source, so
  there is nothing to hand loadstring.
• _G is frozen. table.isfrozen(_G) is true and writes raise. A CLI-loaded module's
  globals table has __index = _G, so no require shim can be placed in the chain before
  the module body runs.
• Symlinks are not followed. A mirror directory of symlinks to the real modules fails
  with could not resolve child component.
• The two resolvers cannot agree on one name. The shell takes the request as the file
  name and demands it end in .luau (luau_host.cpp:2948-2951); the CLI takes it as a stem
  and appends .luau. For a file F the shell needs the request to be F and the CLI needs
  it to be F minus .luau. Different strings, always.

So any file containing require("./x.luau") is unloadable by luau test.luau — entries
included — and that would have taken roughly 2000 of the suite's 2443 lines with it.

What ships instead: one <name>.luau.luau beside each module, six lines, forwarding to
the real file. The CLI finds it for ./x.luau; the shell resolves the same string to x.luau
and never opens the alias. They are not in the watch set either — that is the entry plus
LuauHost::loadedModulePaths(), which is populated only for modules actually required
(luau_host.cpp:3046). They ship as inert bytes in the plugin directory. That is the cost,
and it is the thing to challenge hardest in review.

The half that is achievable from inside the suite is the environment, done after load,
because a module's globals table is a plain writable table with a replaceable metatable:

• chainTo(entryEnv, modules) points each module's globals at the entry that required it,
  which is the chain the shell builds (luau_host.cpp:3013-3022). timew.luau therefore
  sees the panel's stubbed noctalia, and viewmodel.luau sees its frozen os.
• fmt, parse and query are required by both entries, and this suite gets one instance
  where the shell builds two. That is sound only while they read nothing but the standard
  library — so rather than assume it, they run under an environment that raises on any
  global the CLI does not itself define.

Nested require, proved two levels deep. panel.luau requires ./parse.luau, which
requires ./fmt.luau relative to its own directory. Swap Fmt.span for a sentinel and it
has to come back out of Parse.interval, which lives one file further down — one assert
covering both halves at once, that the second hop resolved and that it resolved to the same
cached table rather than a second copy.

A module without its alias is not an assertion failure, it is a load failure. Moving
query.luau.luau aside kills the gate at test.luau:31 with
error requiring module "./query.luau": could not resolve child component "query.luau"
before a single check runs. There is no state in which a module lacks its alias and the
suite still prints ok.

Gates

$ nix shell nixpkgs#luau -c luau test.luau
viewModel: 12 states, 1739 field comparisons
ok
exit=0

$ noctalia plugins lint timew
yuris/timew
  ok

0 errors, 0 warnings
exit=0

Lint follows the new requires: a literal noctalia.getConfig("not_declared_anywhere")
planted in query.luau — two hops below the entry — is reported as
error query.luau:67 reads undeclared setting 'not_declared_anywhere'.

Check count

viewModel: 12 states, 1739 field comparisons before and after. Unchanged: the view model
computes the same fields from the same state, and the twelve states are driven through the
same handlers.

Four checks were added, and each was shown to fail on a planted defect:

new check	planted defect	failure
nested require	parse.luau grows its own span instead of requiring fmt	test.luau:78: parse.luau's require of ./fmt.luau did not land on the fmt module the entry holds
pure-module environment	query.luau reads noctalia	test.luau:62: a module below the host line read the global 'noctalia'
module globals empty	viewmodel.luau drops a local	test.luau:2595: viewmodel.luau declared a global: chartPeak
bar's sync command	drop a 2>/dev/null from bar's copy	test.luau:688: bar's sync command has drifted from Timew.cmd.activeInterval()

One existing check was repaired: the pure-core counter wrapped every entry of Fmt, Parse
and Query in a counting closure, and Parse.UNTAGGED is a string. Wrapping it left the
model comparing every row's tag against a function for the length of the measurement — the
untagged branch switched off underneath the very count that is supposed to be of an
untouched repaint. It now skips non-functions.

Line counts

Before (main):

   249 timew/bar.luau
  1701 timew/panel.luau
  2443 test.luau
  4393 total

After:

   242 timew/bar.luau
    61 timew/fmt.luau
     6 timew/fmt.luau.luau
  1025 timew/panel.luau
    98 timew/parse.luau
     6 timew/parse.luau.luau
    76 timew/query.luau
     6 timew/query.luau.luau
   376 timew/timew.luau
     6 timew/timew.luau.luau
   172 timew/viewmodel.luau
     6 timew/viewmodel.luau.luau
  2600 test.luau
  4680 total

Net +287 (+257 excluding the 30 lines of alias files). panel.luau −676, bar.luau −7,
test.luau +157. A split that only moved code would be net zero; the growth is module
headers, the loader note, and the four new checks. No behaviour changed.

Watch set, and what a leaf edit does

The shell watches the entry plus every module that entry successfully required
(PluginScriptWatcher::rebuild, fed from LuauHost::loadedModulePaths()):

• yuris/timew:panel — panel.luau, fmt.luau, parse.luau, query.luau, timew.luau, viewmodel.luau
• yuris/timew:bar — bar.luau, fmt.luau, parse.luau
• not watched — the five *.luau.luau aliases, plugin.toml, translations/en.json

Editing a leaf reloads every entry that reached it, not just the file. Saving fmt.luau
fires the watch on both entries, each tears its VM down and rebuilds it, and each rebuilds
its own module cache from scratch — so the panel's state table is recreated at its
declared defaults and the bar's active/startEpoch go back to idle until its next
sync. Saving viewmodel.luau reloads the panel only, since the bar never required it. A
module that fails to load does not enter the watch set (recorded only after a successful
load, luau_host.cpp:3044-3047), so a leaf edited into a syntax error stops being watched
and the next fixing save has to come through the entry's own watch.

What a reviewer should challenge hardest

1. The five alias files. They are dead weight in a shipped plugin directory, and they
   exist only so a test runner can load production code. The alternative was losing most of
   the suite. Judge whether that trade is right — the aggregator variant was measured and is
   worse (still five shims, plus a real file, and its collapsing form is a cycle the CLI
   silently accepts and the shell rejects).
2. One module instance where the shell builds two. fmt, parse and query are shared
   between two entries in the suite and would not be in production. The purity environment is
   what makes that sound; if any of those three ever needs a host global, that assumption
   breaks and the suite must grow a second instance it currently cannot build.
3. viewmodel.luau requiring the command layer for two pure values. It is the one edge in
   the graph that is a convenience rather than a necessity.
4. The per-repaint closure in the view model. passesFilter was a module local over
   module state; state is an argument now, so it is one closure per repaint. One allocation
   a second, not one per row — but it is a real change from what the old comment claimed.
5. Timew.DAYS is a display constant living in the command layer. It is one number doing
   two jobs, and it had to land on one side.

`panel.luau` was 1701 lines holding six layers. It is 1025 now and holds only what an entry has to: state, the handlers that move it, the region builders, and the assembly of the tree. ## The graph ```mermaid graph TD fmt[fmt.luau · 61 · pure] parse[parse.luau · 98 · pure] query[query.luau · 76 · pure] timew[timew.luau · 376] vm[viewmodel.luau · 172] panel[panel.luau · 1025 · entry] bar[bar.luau · 242 · entry] parse --> fmt timew --> fmt timew --> parse vm --> fmt vm --> parse vm --> query vm --> timew panel --> fmt panel --> parse panel --> query panel --> timew panel --> vm bar --> fmt bar --> parse ``` **Acyclicity, how it was checked.** The seven files' `require("./x.luau")` strings were extracted and walked with a colouring DFS. Result: acyclic, topological order `fmt < parse < query < timew < viewmodel < panel < bar`; every require target has exactly one alias file and no alias file lacks a requirer. No back-edge was wanted and none was added — `state` travels down as an argument, never up as a require. Note that the *runtime* gate for this is the shell's, not the suite's: `pushRequiredModule` walks `m_moduleStack` and raises `circular require: <full chain>` (`luau_host.cpp:2976-2985`). The bare `luau` CLI does **not** detect cycles — a two-file cycle built in `/tmp` ran clean and exited 0 under it — so a cycle would pass `luau test.luau` and fail to load the plugin. That asymmetry is why the split keeps the graph shallow rather than routing edges through an aggregator. ## Modules | file | lines | contents | reads a host global? | |---|---:|---|---| | `fmt.luau` | 61 | `clock`, `span`, `dayLabel` | no | | `parse.luau` | 98 | `UNTAGGED`, `trim`, `toast`, `unquote`, `duration`, `clock`, `interval` | no | | `query.luau` | 76 | `matching`, `barRoles`, `tagColor`, `barPeak` | no | | `timew.luau` | 376 | `DAYS`/`DAY_WALK_MAX`/`ALL_WALK_MAX`, the parsers, `cmd.*`, the `runAsync` wrappers, `exec` | `noctalia`, inside function bodies only | | `viewmodel.luau` | 172 | `build(state)`, `TAG_COLORS` | `os.time` | | `panel.luau` | 1025 | layout constants, `state`, `apply`/`commit`, fetches, handlers, builders, `render`, `onOpen`/`update`/`onIpc` | yes | | `bar.luau` | 242 | `Config`, tick, `sync`, `render`, `onClick` | yes | Three deviations from the shape in `UPGRADE.md`: **No `config.luau`.** The manifest declares settings on the bar entry only; the panel reads none. `Config` has exactly one caller, duplicates nothing, and extracting it would add a file and a second stubbed environment to the suite for no present reader. U6 gives the panel formatting settings — that is its first real second consumer, and the right task to move it. A module with one caller is a file to open, not a boundary. **`viewmodel.luau` requires `timew.luau`.** It reads exactly two pure values from it: `tagMatches` (the same substring test the walk filters with) and `DAYS` (the chart is as wide as the fetch). It calls nothing impure. Moving `tagMatches` into `query.luau` would cut the edge, but the tracker's own layering puts the view model above the commands, and the move would churn the suite's `T.tagMatches` asserts and the pure-core counter's note for an edge that is a read of two constants. **The three fetch bounds are exported.** `DAYS`, `DAY_WALK_MAX` and `ALL_WALK_MAX` were locals of the one file that both built the commands and drew the chart. They belong with the commands, and the panel needs `DAYS` for `RIGHT_W` and the trend caption. So they live in `timew.luau` and are read off it. The suite's frozen scripts stay frozen literals, and a new assert ties the exported numbers to them — a pair that could not be checked against each other at all before. ## bar.luau: what is shared and what is not The bar is its own entry with its own Luau VM and its own module cache, so a shared module means **each VM loads its own instance of one file** — the sharing is of source, not of state, and neither shared module has any state to share. | helper | ruling | why | |---|---|---| | `formatElapsed` | **share** → `Fmt.clock` | byte-identical body; the two surfaces must render one interval as one string | | `trim` | **share** → `Parse.trim` | byte-identical, and `toast` needs it anyway | | `unquote` | **share** → `Parse.unquote` | byte-identical | | `toast` | **share** → `Parse.toast` | carries two user-visible strings that existed twice: `"timew is not on PATH"` and `"timew failed (exit N)"` | | `"(untagged)"` | **share** → `Parse.UNTAGGED` | the strongest case in the file: a wire value the bar publishes into `timew.tags` and the panel compares with `~=`. It was a literal in each entry | | `elapsed` | **keep** | reads bar's own module state; there is nothing to share | | `Config` | **keep** | see above | | the `sync` shell program | **keep, and pin** | requiring `./timew.luau` for one string would put a bar tick downstream of the whole command layer. It was an *unguarded* duplicate; it is now held against `Timew.cmd.activeInterval()`, the way the `timew start` argv already was | | `timew start` / `timew stop` argv | **keep** | the bar's tag comes from its own `Config` and its stop is a branch of one click handler; already pinned against the panel's | ## The test loader, and why it is not the one that was specified The brief called for a loader that resolves `./x.luau` itself, caches per path, injects stubbed globals and injects itself as `require`. Half of that is impossible on this runtime, and it was checked rather than assumed: - **No `io`.** `type(io)` is `nil` in `luau`. There is no way to read a module's source, so there is nothing to hand `loadstring`. - **`_G` is frozen.** `table.isfrozen(_G)` is true and writes raise. A CLI-loaded module's globals table has `__index = _G`, so no `require` shim can be placed in the chain before the module body runs. - **Symlinks are not followed.** A mirror directory of symlinks to the real modules fails with `could not resolve child component`. - **The two resolvers cannot agree on one name.** The shell takes the request as the file name and demands it end in `.luau` (`luau_host.cpp:2948-2951`); the CLI takes it as a stem and appends `.luau`. For a file `F` the shell needs the request to be `F` and the CLI needs it to be `F` minus `.luau`. Different strings, always. So any file containing `require("./x.luau")` is unloadable by `luau test.luau` — entries included — and that would have taken roughly 2000 of the suite's 2443 lines with it. What ships instead: **one `<name>.luau.luau` beside each module**, six lines, forwarding to the real file. The CLI finds it for `./x.luau`; the shell resolves the same string to `x.luau` and never opens the alias. They are not in the watch set either — that is the entry plus `LuauHost::loadedModulePaths()`, which is populated only for modules actually required (`luau_host.cpp:3046`). They ship as inert bytes in the plugin directory. **That is the cost, and it is the thing to challenge hardest in review.** The half that *is* achievable from inside the suite is the environment, done after load, because a module's globals table is a plain writable table with a replaceable metatable: - `chainTo(entryEnv, modules)` points each module's globals at the entry that required it, which is the chain the shell builds (`luau_host.cpp:3013-3022`). `timew.luau` therefore sees the panel's stubbed `noctalia`, and `viewmodel.luau` sees its frozen `os`. - `fmt`, `parse` and `query` are required by both entries, and this suite gets one instance where the shell builds two. That is sound only while they read nothing but the standard library — so rather than assume it, they run under an environment that **raises** on any global the CLI does not itself define. **Nested require, proved two levels deep.** `panel.luau` requires `./parse.luau`, which requires `./fmt.luau` relative to its own directory. Swap `Fmt.span` for a sentinel and it has to come back out of `Parse.interval`, which lives one file further down — one assert covering both halves at once, that the second hop resolved and that it resolved to the same cached table rather than a second copy. **A module without its alias is not an assertion failure, it is a load failure.** Moving `query.luau.luau` aside kills the gate at `test.luau:31` with `error requiring module "./query.luau": could not resolve child component "query.luau"` before a single check runs. There is no state in which a module lacks its alias and the suite still prints `ok`. ## Gates ``` $ nix shell nixpkgs#luau -c luau test.luau viewModel: 12 states, 1739 field comparisons ok exit=0 $ noctalia plugins lint timew yuris/timew ok 0 errors, 0 warnings exit=0 ``` Lint follows the new requires: a literal `noctalia.getConfig("not_declared_anywhere")` planted in `query.luau` — two hops below the entry — is reported as `error query.luau:67 reads undeclared setting 'not_declared_anywhere'`. ## Check count `viewModel: 12 states, 1739 field comparisons` before and after. Unchanged: the view model computes the same fields from the same state, and the twelve states are driven through the same handlers. Four checks were added, and each was shown to fail on a planted defect: | new check | planted defect | failure | |---|---|---| | nested require | `parse.luau` grows its own `span` instead of requiring fmt | `test.luau:78: parse.luau's require of ./fmt.luau did not land on the fmt module the entry holds` | | pure-module environment | `query.luau` reads `noctalia` | `test.luau:62: a module below the host line read the global 'noctalia'` | | module globals empty | `viewmodel.luau` drops a `local` | `test.luau:2595: viewmodel.luau declared a global: chartPeak` | | bar's `sync` command | drop a `2>/dev/null` from bar's copy | `test.luau:688: bar's sync command has drifted from Timew.cmd.activeInterval()` | One existing check was repaired: the pure-core counter wrapped every entry of `Fmt`, `Parse` and `Query` in a counting closure, and `Parse.UNTAGGED` is a string. Wrapping it left the model comparing every row's tag against a *function* for the length of the measurement — the untagged branch switched off underneath the very count that is supposed to be of an untouched repaint. It now skips non-functions. ## Line counts Before (`main`): ``` 249 timew/bar.luau 1701 timew/panel.luau 2443 test.luau 4393 total ``` After: ``` 242 timew/bar.luau 61 timew/fmt.luau 6 timew/fmt.luau.luau 1025 timew/panel.luau 98 timew/parse.luau 6 timew/parse.luau.luau 76 timew/query.luau 6 timew/query.luau.luau 376 timew/timew.luau 6 timew/timew.luau.luau 172 timew/viewmodel.luau 6 timew/viewmodel.luau.luau 2600 test.luau 4680 total ``` Net **+287** (+257 excluding the 30 lines of alias files). `panel.luau` −676, `bar.luau` −7, `test.luau` +157. A split that only moved code would be net zero; the growth is module headers, the loader note, and the four new checks. No behaviour changed. ## Watch set, and what a leaf edit does The shell watches the entry plus every module that entry successfully required (`PluginScriptWatcher::rebuild`, fed from `LuauHost::loadedModulePaths()`): - **`yuris/timew:panel`** — `panel.luau`, `fmt.luau`, `parse.luau`, `query.luau`, `timew.luau`, `viewmodel.luau` - **`yuris/timew:bar`** — `bar.luau`, `fmt.luau`, `parse.luau` - **not watched** — the five `*.luau.luau` aliases, `plugin.toml`, `translations/en.json` Editing a leaf reloads every entry that reached it, not just the file. Saving `fmt.luau` fires the watch on both entries, each tears its VM down and rebuilds it, and each rebuilds its own module cache from scratch — so the panel's `state` table is recreated at its declared defaults and the bar's `active`/`startEpoch` go back to idle until its next `sync`. Saving `viewmodel.luau` reloads the panel only, since the bar never required it. A module that fails to load does **not** enter the watch set (recorded only after a successful load, `luau_host.cpp:3044-3047`), so a leaf edited into a syntax error stops being watched and the next fixing save has to come through the entry's own watch. ## What a reviewer should challenge hardest 1. **The five alias files.** They are dead weight in a shipped plugin directory, and they exist only so a test runner can load production code. The alternative was losing most of the suite. Judge whether that trade is right — the aggregator variant was measured and is worse (still five shims, plus a real file, and its collapsing form is a cycle the CLI silently accepts and the shell rejects). 2. **One module instance where the shell builds two.** `fmt`, `parse` and `query` are shared between two entries in the suite and would not be in production. The purity environment is what makes that sound; if any of those three ever needs a host global, that assumption breaks and the suite must grow a second instance it currently cannot build. 3. **`viewmodel.luau` requiring the command layer** for two pure values. It is the one edge in the graph that is a convenience rather than a necessity. 4. **The per-repaint closure in the view model.** `passesFilter` was a module local over module state; `state` is an argument now, so it is one closure per repaint. One allocation a second, not one per row — but it is a real change from what the old comment claimed. 5. **`Timew.DAYS` is a display constant living in the command layer.** It is one number doing two jobs, and it had to land on one side.

yuris added 1 commit 2026-09-01 16:24:53 +00:00

refactor(timew): split panel.luau into modules, teach the suite to load them ... 8e643984f3

panel.luau was 1701 lines holding six layers. It is 1025 now and holds the
entry's own work: state, the handlers that move it, the region builders and
the assembly of the tree. Under it, one file per layer, arrows pointing one
way:

  fmt.luau        formatting          pure, no requires
  parse.luau      text -> data        pure, requires fmt
  query.luau      derived view logic  pure, no requires
  timew.luau      commands + parsing  requires fmt, parse
  viewmodel.luau  state -> model      requires fmt, parse, query, timew

The view model reads `state` as an argument rather than a module local two
hundred lines above it, which is what made it a file rather than a section.
DAYS, DAY_WALK_MAX and ALL_WALK_MAX moved to timew.luau -- they bound a fetch
-- and are exported, so the chart's width and the panel's call sites read the
same number the builders default to.

bar.luau requires fmt.luau and parse.luau. That retires four copies: trim,
unquote, formatElapsed (Fmt.clock) and toast, and with toast the two
user-visible strings it holds, and the (untagged) sentinel the bar publishes
onto the wire and the panel compares. Config stays in bar.luau -- one caller,
nothing duplicated -- and so does the active-interval shell program, which is
now pinned against Timew.cmd.activeInterval() instead of going unchecked.

test.luau loads the same panel the shell loads. The two require resolvers are
mutually exclusive by construction: the shell takes the request as the file
name, the bare luau CLI appends .luau to it, so for any one file on disk the
two want different strings. The CLI has no io library and a frozen _G, so the
suite can neither read a module's source nor inject a require of its own.
What it can do is the environment half, after the fact: a `<name>.luau.luau`
beside each module is what the CLI finds and forwards, and the suite rebuilds
the shell's module-to-entry globals chain over the loaded tables. The pure
three run under an environment that raises on any global the CLI does not
itself define.

Coverage is unchanged where it existed and wider where the split earned it:
the nested hop is proved by swapping Fmt.span for a sentinel and finding it
in Parse.interval; every module's globals table is asserted empty; the pure
core counter no longer wraps Parse.UNTAGGED and silently disables the
untagged branch for the length of its own measurement.

viewModel: 12 states, 1739 field comparisons -- the same numbers as main.

yuris reviewed 2026-09-01 18:10:00 +00:00

yuris left a comment

Copy link

Verdict: APPROVE

Reviewed by the integrator after two delegated reviewers hung without producing output. Every
number below is derived here, not taken from the PR body.

Verified

Graph, independently extracted from the require strings rather than read from the PR:
fmt -> (none), query -> (none), parse -> fmt, timew -> fmt parse,
viewmodel -> fmt parse query timew, panel -> all five, bar -> fmt parse. Acyclic;
topological order as claimed. Two leaves, one entry per widget.

The split is a pure move, with two declared exceptions. Multiset diff of non-comment,
non-blank code lines between 11d8841:timew/panel.luau and the concatenation of the new panel
plus all five modules: 16 lines only in baseline, 44 only in new. All but two classes are
UNTAGGED -> Parse.UNTAGGED qualification and module scaffolding (local M = {}, return M,
the export lines). The two real restructures:

1. viewModel() reading the module-level state became Viewmodel.build(state) taking it as
   a parameter. Forced by the boundary - a module cannot see the panel's local.
2. The filter predicate moved from an unconditionally-defined passesFilter to one constructed
   only when state.mode ~= "all". The body is byte-identical
   (return Timew.tagMatches(row.tag, state.filter)) and the value handed to Query.matching
   is the same in both modes, so this is equivalent; it also stops allocating a closure that
   all-time scope discards.

Neither is a behaviour change, but both are rewrites rather than moves and the PR body should
say so - on this project "the move is provably a move" is the standard.

Settings-drift protection survived the split. Planted getConfig("not_declared_anywhere")
in query.luau, two hops below the entry:
error query.luau:21 reads undeclared setting 'not_declared_anywhere' -> 1 error. Restored,
back to 0/0. Lint follows requires.

Gates: 12 states, 1739 field comparisons / ok, and 0 errors, 0 warnings. Check count
unchanged from main, as claimed.

Mutations

Five against cross-module boundaries, each restored byte-identically and each verified to have
actually applied (my first pass produced two false survivors from patterns that missed - worth
recording, since an unapplied mutation reports as a survivor and looks like a finding):

#	Mutation	Result
M3	Parse.UNTAGGED sentinel changed	killed
M4	Query.matching drops its predicate argument	killed
M5	Fmt.clock renamed, breaking a cross-module call	killed
M6	exported Timew.DAYS halved	killed
M7	pure module gains a host call at load	killed

The alias scheme

Accepted. The constraint is real and I verified its shape independently at
luau_host.cpp:2947-2951: the shell requires the request to end .luau and resolves it as
written, while the bare CLI appends .luau - two different strings for one file, so no single
require path loads under both. With no io and a frozen _G there is no in-suite loader, and
the aggregator variant I proposed was measured and rejected on better evidence than I had: it
raises the file count and, in the form that would cut it, creates a cycle the CLI runs clean
and the shell rejects. A shape that passes the gate and fails to load is worse than five inert
files.

A user installing this plugin is not harmed: the alias files are never required, so the shell
neither opens nor watches them. The cost is five files of noise in the plugin directory and a
drift surface if a module is renamed without its alias - and that case fails the suite at load
rather than silently, which is the right failure.

Advisory

1. The PR body calls the split a pure move; name the two restructures above explicitly.
2. bar.luau and panel.luau each keep their own toast; the PR shares fmt/parse but this
   user-visible string still exists twice. It is the exact drift hazard sharing was meant to
   close - worth a line in the body saying why it stayed, or closing it in U7.

**Verdict: APPROVE** Reviewed by the integrator after two delegated reviewers hung without producing output. Every number below is derived here, not taken from the PR body. ## Verified **Graph, independently extracted** from the `require` strings rather than read from the PR: `fmt -> (none)`, `query -> (none)`, `parse -> fmt`, `timew -> fmt parse`, `viewmodel -> fmt parse query timew`, `panel -> all five`, `bar -> fmt parse`. Acyclic; topological order as claimed. Two leaves, one entry per widget. **The split is a pure move, with two declared exceptions.** Multiset diff of non-comment, non-blank code lines between `11d8841:timew/panel.luau` and the concatenation of the new panel plus all five modules: 16 lines only in baseline, 44 only in new. All but two classes are `UNTAGGED -> Parse.UNTAGGED` qualification and module scaffolding (`local M = {}`, `return M`, the export lines). The two real restructures: 1. `viewModel()` reading the module-level `state` became `Viewmodel.build(state)` taking it as a parameter. Forced by the boundary - a module cannot see the panel's local. 2. The filter predicate moved from an unconditionally-defined `passesFilter` to one constructed only when `state.mode ~= "all"`. The body is byte-identical (`return Timew.tagMatches(row.tag, state.filter)`) and the value handed to `Query.matching` is the same in both modes, so this is equivalent; it also stops allocating a closure that all-time scope discards. Neither is a behaviour change, but both are rewrites rather than moves and the PR body should say so - on this project "the move is provably a move" is the standard. **Settings-drift protection survived the split.** Planted `getConfig("not_declared_anywhere")` in `query.luau`, two hops below the entry: `error query.luau:21 reads undeclared setting 'not_declared_anywhere'` -> 1 error. Restored, back to 0/0. Lint follows requires. **Gates:** `12 states, 1739 field comparisons / ok`, and `0 errors, 0 warnings`. Check count unchanged from `main`, as claimed. ## Mutations Five against cross-module boundaries, each restored byte-identically and each verified to have actually applied (my first pass produced two false survivors from patterns that missed - worth recording, since an unapplied mutation reports as a survivor and looks like a finding): | # | Mutation | Result | |---|----------|--------| | M3 | `Parse.UNTAGGED` sentinel changed | killed | | M4 | `Query.matching` drops its predicate argument | killed | | M5 | `Fmt.clock` renamed, breaking a cross-module call | killed | | M6 | exported `Timew.DAYS` halved | killed | | M7 | pure module gains a host call at load | killed | ## The alias scheme Accepted. The constraint is real and I verified its shape independently at `luau_host.cpp:2947-2951`: the shell requires the request to end `.luau` and resolves it as written, while the bare CLI appends `.luau` - two different strings for one file, so no single require path loads under both. With no `io` and a frozen `_G` there is no in-suite loader, and the aggregator variant I proposed was measured and rejected on better evidence than I had: it raises the file count and, in the form that would cut it, creates a cycle the CLI runs clean and the shell rejects. A shape that passes the gate and fails to load is worse than five inert files. A user installing this plugin is not harmed: the alias files are never required, so the shell neither opens nor watches them. The cost is five files of noise in the plugin directory and a drift surface if a module is renamed without its alias - and that case fails the suite at load rather than silently, which is the right failure. ## Advisory 1. The PR body calls the split a pure move; name the two restructures above explicitly. 2. `bar.luau` and `panel.luau` each keep their own `toast`; the PR shares `fmt`/`parse` but this user-visible string still exists twice. It is the exact drift hazard sharing was meant to close - worth a line in the body saying why it stayed, or closing it in U7.

yuris merged commit 084f20914d into main 2026-09-01 18:10:13 +00:00

yuris deleted branch upgrade/u4-modules 2026-09-01 18:10:14 +00:00

yuris referenced this pull request from a commit 2026-09-01 18:10:14 +00:00

refactor(timew): split panel.luau into modules, teach the suite to load them (#14)

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

Subscribe

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!14

No description provided.