refactor(panel): collapse the module state into one table behind a single commit #10
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "refactor/t6-state"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Wave 6, the last structural one: the panel's scattered mutable module locals
become one
statetable, and every repaint goes through a singlecommit.Review #47 fixup (
cd3395b). Four findings, all applied; the panel change iscomments only. Two of them were claims this description and
test.luau's commentsmade that the code did not back: the bypass invariant was grep-only where the
suite said it was test-enforced, and PR #8's globals snapshot never caught the
dropped
localits comment advertised (it was taken after the model had alreadycreated the global, so the loop compared the table against itself). Both are
closed and mutation-proved.
adoptState'sapply— the one site the repainttable never reached — is now measured and driven in the suite. The invariant awk
block below is re-quoted at the current commit.
What moved
One
statetable. The twenty locals named in the ticket, pluselapsedText— it is view state by every test that matters (mutated by a watch, read by the
model, shown on the tree), and leaving it outside would have meant
viewModelreading
stateand one stray local.pokeandwatchingstay module localsand are commented as such: neither reaches the view model, so neither is a reason
to repaint, and putting them in
statewould create fieldscommitmust neverbe handed.
Constants are untouched:
ROWS_MAX,DAYS,RECENT_MAX,PAGE_BUTTONS, thewidths and heights,
UNTAGGED,TAG_COLORS.Two functions own the table.
apply(patch)writesstateand never paints. Six call sites, each one a paththat must not paint where it writes:
loadDay(the day's walk lands therepaint — a bar click painted once before this change and paints once now),
fetchDays(applies the totals, reloads the day, commits once),adoptState(insiderefresh, whose callers paint later or not at all —measured separately, since the repaint table below never reaches it),
onOpen(defaults applied so the refetches are issued against day scope andpage 1, then one commit at the end), and
onSearchChange/onDraftChange(the input widget is already showing the keystroke; neither painted before).
commit(patch)applies and paints exactly once. Eighteen call sites. It is theonly caller of
renderin the file.One semantic narrowing against the twenty assignments
applyreplaces, nowwritten on its comment: it iterates
pairs(patch), so a patch field whose valueis
nilis not written where the old assignment would have cleared the field.No producer can emit one today (
parseActivereturns literals orParse.unquote, which falls back to its argument; every other parser returns atable or a string), and the closed-field-set assert covers misspelling rather
than this. A field that needs clearing gets its zero value in the patch, the way
watchState's idle branch does (patch.startEpoch, patch.tags = 0, "").Two functions rather than one because the alternative is worse in the direction
the ticket cares about: forcing those six paths through
commitadds repaints (akeystroke would rebuild the whole tree per character). The invariant the tests
and the gate pin is the one that matters —
state.<field> =appears insideapplyandcommitand nowhere else:That grep sees the source shape and nothing else, and one bypass shape does not
have that shape:
commitpublishesstate.shownas the model's ownwindowRowstable rather than a copy, so a region builder that mutated the modelin place would move state without ever writing
state.<field>. Nothing does(
grep -nE '\bm\.[A-Za-z_]+ *=[^=]|table\.(insert|remove|move|sort)\(state': nomatches, and main aliased the same table identically), and it is now written on
the
statetable's own comment. The complementary check — that every pathpublishes what it painted — is in the suite; see "New asserts" below.
renderwrites nothing. It takes the model as an argument.commitbuildsone model per repaint, publishes
state.shown/state.matchingoff it at thesame point and in the same order
renderused to, hands the model down, andclears
state.focusOnceafter the paint.focusOncebelongs tocommitbecause the flag's contract is "the repaintthat carries it to the tree consumes it" — that was
render's job only becauserenderwas the repaint. Now that repaints are explicit and singular, therepainter consumes it, which is the same point in the same order. The observable
asymmetry test.luau already pinned (
focus == trueon onOpen's tree,focusOnce == falseby the time the model can be called) is unchanged and stillpasses; only the assert's message changed.
Call-site count
The ticket says 45. That is
grep -c render— every line mentioningrender,comments included. Actual
render()call statements on main: 19. After:1 (inside
commit), reached through 18commitand 6applycall sites.Repaints, per path, before and after
Measured, not reasoned about: main's
panel.luauand this branch's are loaded inone luau process, each with its own global table and identical stubs, driven
through the same path, with
panel.rendercounted.cmdis commands queued orlanded (negative = landed). Harness:
/tmp/pr9/h/repaints.luau.The two paths that were paying twice, named:
setFiltercommitted the tagand painted under the old scope, then
onSearchSubmitmoved the scope toall-time and painted again — the first of those two trees showed the new chip
over a day-scope list nobody asked for. Now the scope rides in
setFilter'spatch: 2 → 1.
main also issued the all-time walk twice:
setFilterissued it (scope wasalready
all), thenonSearchSubmitissued it again for the same tag.2 paints → 1, and 5 commands → 3 — the command dropped is a 200-id DOM
walk, the most expensive thing this panel asks timew for.
Everything else is identical, including the deliberate zeros: a bar click still
paints when its intervals arrive rather than before (
loadDayapplies, the walkcommits), a keystroke still paints nothing, and
day 99— the route to thedeferred backwards-range bug — still returns without painting or fetching.
The async landings are not merged and must not be:
refresh's five fetches landat five different times, so they are five logical changes and five repaints, on
both sides. Merging them means holding a paint until the slowest walk returns.
One
applysite this table does not cover, and its own measurement. Theharness's
noctalia.state.getreturnsnil, soadoptStatetakes itsa == nilearly return on all 38 steps above and the
applyunder it never runs — "0 pathsworse" is true but says nothing about that site. Measured with a bar widget that
actually publishes:
Committing there costs +1 paint per open and +1 per mutating command, and the
first tree is the same either way. So the site is justified by measurement, not by
argument — and that path is now driven in the suite rather than only here (see
"New asserts").
Timing
Same harness as PR #9's review (
/tmp/pr9/h/cost3.luau), worst state: 200all-time rows, page 4, min of 9 interleaved rounds x 500 calls, three runs.
viewModel()Per-repaint cost is unchanged: the delta is ±1% and changes sign between runs
(-1.0%, +0.9%, -0.6%), i.e. noise. 0.20–0.21% of the 25 ms per-handler budget on
both sides. The patch table
commitallocates does not register against a treeof 104 nodes.
The path that lost a repaint, timed end to end (Enter in the search field from
all-time scope, min of 9 x 500, three runs):
-47% on that handler, and one fewer 200-id shell walk behind it.
Tree identity: 32 states, 0 differences
PR #9's harness (
/tmp/pr9/h/treediff.luau), unchanged except for the branchname in its banner. Both panels in one process, own global tables, identical
stubs, props key-sorted, flattened one line per node, compared byte for byte.
The async guards
PR #6's reproduction, re-run: issue the all-time walk under
focus, change thetag to
emailmid-flight, land the fresh walk, then land the stale one. Theguards moved from above the assignment to above the
commit, which is the sameplace semantically — but a refused patch is now also a repaint that never
happens, so there is a second, sharper thing to assert:
The pre-existing content asserts (test.luau:659, :663) still pass unchanged. The
new count catches a failure mode they cannot see: a guard that drops the patch
but paints anyway. Mutation-proved below.
How the test suite still reaches state
The ticket's premise needs one correction, and it is load-bearing for reviewing
this PR: test.luau never drove state by assignment, because it never could.
panelEnv = getfenv(panel.Fmt.clock)is the module's globals table. The statelocals were
local, i.e. upvalues, and Luau ships nodebug.getupvalue— thesuite says so itself (:1030 on main). It drives state through the handlers the
shell calls (
onOpen,onIpc,onRowN,onPageN,update, the fournoctalia.state.watchcallbacks) and reads it back off the rendered tree andpanel.viewModel().So the mechanism needs no adaptation to keep working, and the 14 states and 1485
field comparisons pass unchanged. What did change:
stateis now exported (return { ..., state = state }, the table alreadydocumented "exported for test.luau only"). One value, so for the first time the
suite can read the panel's whole mutable surface directly. It is read there,
never written: writing it would be the one bypass of
commitin the repo.PR #8's env-key snapshot was moved, because it never caught what it claimed
to. It is supposed to catch a
localdropped insideviewModellanding anew global in the module's globals table — a class the state export cannot see
(a name that never reached the table is not a field of it). It did not, on this
branch or on main:
panelEnvis the module's own globals table, taken attest.luau:492, so a global the model writes exists from the model's first callonward, and a snapshot taken further down (beside the viewModel section, after
the panel has been driven) had already recorded the name. The loop at the bottom
compared the table against itself. Dropping
localfromchartPeakinsideviewModelprintedokon both sides.It now snapshots immediately after the
panelEnv.panelstub, before the firsthandler runs, with
os— the only key the table legitimately gains afterwards,from the frozen-clock stub — seeded by hand. Drop that seed and the pristine
suite fails on
os, which is what keeps the snapshot honest about being early.stateis alocalcreated at load time, so it still adds no key.The hole was PR #8's rather than this wave's, but this wave rewrote the comment
that asserted it, so it is fixed here.
The other half is now asserted by value, which is new capability rather
than a replacement: dump
state, callviewModel()five times, dump again.Plus the landed values by name (
#state.recent == 4,state.total,state.tags, ...), because a before/after comparison alone passes a model thatflattens a field — the first call flattens it, every later call finds it
already flat, and idempotence is not purity. That hole was real: a
state.recent = {}planted inviewModelpassed the whole suite until thoseasserts existed.
Asserts whose shape changed
renderto the end ofcommit; both asserts still pin the same observableoslocalits comment claimed; see aboveNo assert was weakened or removed. 354 lines added.
New asserts
commitpaints exactly once per call, path by path, wrapping the stub'spanel.renderwith a counter: 1 for a page click, a scope click, a filter set,Enter in either field, each fetch landing, each tick while tracking; 0 for a
keystroke, a bar click (its walk pays),
day 99, and a tick while idle.Nothing bypasses
commit, in the two halves that are reachable from here.A path that mutates without committing paints 0 and the tree falls behind the
state — the counter above sees that. A path that writes
statedirectly andpaints anyway renders a correct tree, so neither the counter nor any tree
comparison in the file can see it; what it drops is the rest of
commit. Soafter every counted paint,
paintsForalso asserts thatstate.shownandstate.matchingare the painted model's own window and total and thatstate.focusOncewas cleared. That makes all 23 counted paths bypass checks.The mutant that motivated this —
setFilter'scommit(patch)replaced by thefour
state.<field> =writes plusrender(viewModel())— used to printok,and is not benign: it leaves the previous window published, so clicking the one
row left after filtering to
adminmoves the chip tofocus, and 'next page'reports "page 2 of 1, showing 9-8 of 5".
What stays out of the suite is the source shape (
state.<field> =only insideapplyandcommit). The luau CLI ships noiolibrary — test.luau notes thesame at
:743for en.json — so that grep runs in the gate; its output is at thetop of this description, and the shape it cannot see is noted there too.
adoptState's success path is driven, so the sixthapplyis counted too.It was the one panel path the counter never reached:
noctalia.state.getreturns
nilin this harness, so thea == nilearly return fired on everyopen. With the bar widget publishing,
refreshadopts instead of issuingfetchActive, and the assert is that adopting costs no tree of its own — onepaint for the open, the adopted interval already on it (the action button reads
onStopand the subject line reads the adopted tag, both before any fetch haslanded), then the four landings one paint each.
adoptStatecommitting insteadmakes that open paint twice.
commitpublishes the model's own window and match count — assertedagainst
viewModel()rather than a literal, since the claim is that they arenot a second derivation.
The field set of
stateis closed.applywritesstate[field], so apatch with a misspelled field would grow a silent twenty-second field while the
model went on reading the old one. This is the hazard the refactor introduces
in exchange for a greppable state surface, so it gets its own assert.
A repaint of unchanged state moves no field, and a repaint after the open
one does not re-request focus (read off the second tree after a keystroke,
since a flag set on the way out of one paint only reaches the next).
The stale-landing guards cost zero repaints (above).
Four commands for one Enter in the search field, not six.
Mutation proof
Every new assert broken, run, restored,
sha256compared (all restoresbyte-identical; the suite is green on the pristine tree before and after).
Line numbers below are at
84d4201, the commit these were measured at —git checkout 84d4201reproduces them verbatim. The review fixup that followsadds ~104 lines to
test.luauand moves them; the assert message in each row isthe stable identifier. The fixup's own mutations are in the table after this one.
commitcallsrendertwicetest.luau:1391: Enter in the search field painted 2 times, not 1onScopeDayapplies instead of committingtest.luau:1391: the scope switch back to this day painted 0 times, not 1onSearchSubmitrestored to main's two-paint shapetest.luau:1391: Enter in the search field painted 2 times, not 1test.luau:1391: a walk issued under a tag the panel has left painted 1 times, not 0searchKey(nothing else reads it)test.luau:1509: a patch grew state a field no declaration has: searchKeyycommitpublishes a stale match counttest.luau:1545: the published match count is not the model's: 4 vs 3fetchTotal's patch namesallTotaltest.luau:1534: the day total and the live interval are the fetches' own, after five model callsviewModelclearsrecentafter building the chipstest.luau:1529: a fetch landing lost its rows: recent 0, days 3, dayRows 3focusOncetest.luau:1574: a repaint after the open one must not re-request focusapplysilently drops thedraftfieldtest.luau:1573: the keystroke reached state without a repaint of its ownrenderwrites state again (draftKey)test.luau:869: idle, nothing fetched: draftKey -- "tag-26" vs "tag-27"(pre-existing comparison)test.luau:636(PR #6's assert)test.luau:640(PR #6's assert)viewModelwritesstate.matchingtest.luau:1207(pre-existing pager assert)commitstops publishing the windowtest.luau:692(pre-existing row-click assert)Review fixup #47, at
cd3395b:setFilterbypassescommit(fourstate.<field> =writes +render(viewModel()))test.luau:1424: Enter in the search field: the published match count is not the painted model'sok, exit 0 — the gate's awk was the only thing that saw itlocaldropped fromchartPeakinsideviewModel(panel.luau:1111)test.luau:1687: viewModel() wrote a module global: chartPeakok, exit 0, on this branch and on mainenvBefore.osseed removed, pristine paneltest.luau:1686: viewModel() wrote a module global: osadoptStatecommits instead of applying (panel.luau:685)test.luau:1415: an open with the bar widget publishing painted 2 times, not 1state.getreturnednilon every openadoptStatedrops the adopted tag (panel.luau:688)test.luau:1650: the interval adopted off the bar widget reached state without a tree of its ownstate.recent = {}planted atviewModel's tailtest.luau:1568: a fetch landing lost its rows: recent 0, days 3, dayRows 3Honest note on that split: five of these are caught by the pre-existing 1485
comparisons before the new asserts run. That is belt and braces, not redundancy —
the counter catches what the comparisons structurally cannot (a paint that
happens or does not happen with the correct tree either way), and the comparisons
catch what the counter cannot.
Gate
Non-goals held
bar.luauuntouched. No layout, geometry, handler name, settings key, IPC eventor translation key changed — the 32-state byte-for-byte tree identity is the
proof. The deferred
listOffsetbackwards-range bug is still reproduced, notfixed:
state 13of the suite still reads17-16 / 12, and theday 99routeinto it still paints nothing.
What to screenshot after merge
The two paths whose intermediate frame changed are the only visual difference
in the PR, and both are improvements you can only see live:
a day-scope list under the new chip before switching to all-time. Should now
go straight to the all-time list, one frame.
walk fired.
Then the states where nothing should have changed at all:
--:--,--, focus in thetag field (the focus request is the one thing
commitnow consumes).move once a second and match the bar widget.
Nothing tracked, dated scopebutton, and the selection should move when the list does, not before.
noctalia msg plugin yuris/timew:panel all day 99from a painted page 4, thenclick the button painted "2" — the deferred bug, unchanged.
state.shownpublication finding 1 is about. On a correct build the chip doesnot move; the bypass mutant moved it to
focusand made 'next page' report"page 2 of 1, showing 9-8 of 5".
onOpen'sapplyandadoptState'sapplyboth cashing in on the samecommit: the caret lands inthe tag field exactly once and the adopted interval is on that first tree.
Verdict: APPROVE
Re-derived rather than re-read: the invariant grep, the PR #6 race, the 32-state
tree diff, the repaint counter, the timing, and every one of the six
applysites individually. The panel's behaviour is unchanged, the guard survived the
move,
focusOncelands on the same tree, and one of the two "wins" is a realbug fix. Four findings, none blocking; two are about what the tests and the
description claim rather than about the panel.
Harnesses:
/tmp/pr10/h/(common.luaumount +order.luau,race.luau,bypass.luau,focus.luau,adopt.luau), driven againstpmain.luau/pbranch.luaurebuilt fromgit show main:andgit show refactor/t6-state:--cmpsays both are byte-identical to the copies your harness used.Findings
1. The bypass invariant is grep-enforced, not test-enforced -- and the suite says otherwise (P2)
test.luau:1388--paintsForThe new section's second claim is "Nothing bypasses
commit: same counter,from the other side". The counter sees only the half where a path mutates and
fails to paint. The other half -- a path that writes
statedirectly andpaints anyway -- passes the whole suite, because the tree it renders is correct.
What it silently drops is the other two things
commitdoes at:348and:354.My mutant,
setFilter'scommit(patch)replaced by the writes it stands for:The gate catches it. The suite does not:
It is not a benign mutant.
commitalso publishes the window, and without thatthe slice
onRowNindexes is the previous one (/tmp/pr10/h/bypass.luau):The tree diff cannot see it either -- I ran
treediff.luauagainst this exactmutant and it reports
tree-identical, correctly: the trees are identical, thestate the next click reads is not.
I accept that the suite cannot self-grep; I verified the reason rather than
taking
test.luau:743on trust:No
io, noos.getenv, anddebugisinfo/tracebackonly, so there is noroute to the file's text. Fine. But then the claim in the section header has to
shrink to what the counter actually proves.
Two ways to close it, and the second is cheap enough that I would rather have it
than the disclaimer. Fold
commit's own three writes intopaintsFor, whichalready holds the painted state at exactly the right moment:
Applied, on the pristine tree and on the mutant:
That turns your 23 counted paths into 23 bypass checks for ten lines and no new
mechanism. Whatever you do with it, the grep in the gate is still the only thing
that sees the source shape, and the section comment should say so rather than
say "nothing bypasses
commit".One bypass shape neither the grep nor the suite covers, worth a sentence in the
statecomment since the whole design rests on that grep:state.shownism.windowRows, the same table, so a builder that mutated the model would mutatestate without ever matching
state.<field> =. Nothing does today (grep -nE '\bm\.[A-Za-z_]+ *=[^=]|table\.(insert|remove|sort)\(state'on the branch: nomatches), and main aliased it identically. Not introduced here; just the one
hole in the invariant's shape.
2. The rewritten globals-snapshot comment claims coverage the snapshot does not have (P2)
test.luau:1030-1041The comment this PR rewrites now says the table "answers half of that -- an
accidental global, from a
localdropped inside the model, lands the name inhere", and the description repeats it as "PR #8's env-key snapshot is untouched
and still meaningful. It catches a dropped
localinsideviewModel". It doesnot catch it, on this branch or on main.
The global is real -- it just predates the snapshot.
panelEnvis taken at:492and the panel is driven from there on, soviewModelhas run somehundreds of lines before
envBeforeis filled at:1038:(probe = one
printinserted immediately abovelocal envBefore = {}, againstthe mutant.) So the loop at the bottom can only ever catch a global whose first
write happens after
:1038, which forviewModelis none of them.The fix is to snapshot before the first handler runs rather than before the first
open(). The only key the panel's globals table legitimately gains after thestubs is
os, added by thepanelEnv.osstub further down, so it is seeded byhand:
moved to sit immediately after
panelEnv.panel = { ... }(:535), with theoriginal block at
:1038-1041deleted. Verified both directions:The hole is PR #8's, not this wave's -- but this wave rewrites the comment that
asserts it and repeats the claim in the description, and comments are
load-bearing here. Worth noting that the other half is genuinely new and
genuinely works:
dump(panel.state)either side of five calls fails as advertised(
state.recent = {}planted inviewModel->test.luau:1529), so after thisfix both halves would actually be covered.
3. The repaint harness never reaches
adoptState'sapply(P3)timew/panel.luau:663-672repaints.luau'snoctalia.state.getreturnsnil, soadoptStatetakes thea == nilearly return on all 38 measured steps and theapplyunder review isnever executed. "0 paths worse" is true, but it does not cover this site --
routing it through
commitchanges nothing in that table:The conclusion survives; I re-measured it with a bar widget that actually
publishes (
/tmp/pr10/h/adopt.luau):+1 paint per open and +1 per mutating command. So the site is justified -- but by
this measurement, not by the one in the description. Either give
repaints.luaua publishing
state.get, or say in the PR body thatadoptStateis arguedrather than measured.
4. The quoted
awkoutput in the description is two lines off (P3)timew/panel.luau:322-328The description's invariant block reads
324/346/352. At84d4201thelines are
326/348/354:Same three sites, same conclusion; it is the one piece of output in the
description that is not reproducible verbatim, and it happens to be the PR's
central evidence. It is not a stale commit either -- the branch is one commit,
and
state[field] = valueis at326in it.Verified
The invariant holds, including the forms a naive grep misses.
state[field] =only at
:326;state.<field> =only at:348and:354. Norawsetanywhere in the repo; no
state["literal"] =; nostate.X[...] =; notable.insert/remove/move/sort(state...); no alias binding the table to anothername (
state = stateat:1689is the export); onerender(call statement, at:349.grep -c 'commit('= 19 minus the definition = 18 commit sites, andsix
applysites at666 / 734 / 761 / 985 / 999 / 1614. Your "19 on main" isright too:
awkover non-comment lines onpmain.luaugives exactly 19render()statements at
618 622 626 633 642 649 659 671 696 715 722 767 852 859 870 916 927 1547 1555.Async ordering: the PR #6 race does not come back, and the split adds no new
one. Reproduction re-run per PR #6 (
/tmp/pr10/h/race.luau), all-time underfocus, filter toemailmid-flight, stale walk last:R2 lands them in the other order, R3 races two day walks, R4 lands a stale
dayTotals, R5 checks a keystroke reaching the tree on the next unrelatedrepaint, R6 checks
day 99-- every one identical on both sides, includingthe two that are wrong on both (R3 leaves day 2 selected showing day 3's rows;
R4 lands the stale bars,
fetchDaystaking nowantedguard). Pre-existing,byte-for-byte the same shape as main, not this PR's to fix -- but R3 is the
sharper of the two and is the same class PR #6 closed for
fetchAll.No
applysite leaves state unpainted longer than main did. I compared thefull
paint | commandinterleaving per handler rather than the state after it(
/tmp/pr10/h/order.luau, 24 scenarios). Nine streams differ; on eight of themthe commands are identical and the branch's paint lands earlier:
The one command-stream difference is the win, and it is a bug fix, not a
refactor -- worth saying so plainly in the description:
main issues the 200-id DOM walk and its range total twice for one Enter.
That is a duplicated
ALL_WALK_MAX-bounded shell walk on the panel's mostexpensive query, on a keypress. Independent of the repaint story.
The six
applysites, ruled on individually. Each converted tocommitinturn and re-run through
repaints.luau(baseline: branch 37 paints, 0 worse):commitloadDay:761apply. A bar click would paint before its intervals exist, andday 99would paint a day the chart has no entry for.fetchDays:734apply. The totals must land beforeloadDaycan index them; committing splits one landing into two trees.onSearchChange:985apply. One whole-tree rebuild per character.onDraftChange:999apply. Same.onOpen:1614apply. Two trees on every open.adoptState:666apply-- but see finding 3; measured properly it is +1 per open and +1 per mutating command.6/6 genuinely justified, so the two-function split earns itself: this is not
applyexisting becausecommitfelt too blunt once.refresh's five landingsstaying five repaints is right for the same reason -- merging them means holding
the tree until the slowest walk returns. The one thing I would still write down
is that
applyiteratespairs(patch), so a patch field whose value isnilissilently not written where the twenty assignments it replaces would have
cleared the field. No producer can emit
niltoday (parseActivereturnsliterals or
Parse.unquote, which falls back to its argument; every otherparser returns a table or a string), so there is nothing to fix -- but the
closed-field-set assert covers misspelling and not this, and it is the one
semantic narrowing the rewrite makes.
focusOnceis carried by the same tree and consumed once(
/tmp/pr10/h/focus.luau, focus-per-paint recorded across the whole open):F3 and F4 are the two hosts that could paint between
apply({focusOnce = true})and
onOpen'scommit(); both sides move the flag to the same tree at the sameindex. And the asserts that pin it are not vacuous --
onOpenapplyingfocusOnce = falsefailstest.luau:1059,commitnot clearing it failstest.luau:1060.Tree identity and cost reproduce.
pmain.luau/pbranch.luaurebuilt fromgit show,cmp-identical to yours:Not vacuous either:
ROWS_MAX 8 -> 6reportsnode COUNT main 103 branch 91,and deleting
commit'sstate.focusOnce = falsereports the input'sfocus=boolean:trueagainst main'sfalse. Timing over three runs is noise witha sign flip, matching your claim:
-1.3% / -0.5% / +1.1%,0.21% -> 0.21%ofthe 25 ms budget.
The suite still drives everything it drove. The
panelEnv.*handler set is astrict superset -- nothing dropped, five added directly (
onClearFilter,onDraftChange,onScopeDay,onSearchChange,onSearchSubmit); one assertline changed and it is the focus message. And with
apply/commitinstrumentedto log every field write over a full run, all 21 declared fields move:
elapsedTextat 1 is thin, but it is the watch's only writer on both sides.Your new asserts fire where you say. Independently re-run:
viewModelclearing
recent->:1529; a patch misspelling a field ->:1509;commitpainting twice ->
:1391.Gate, from the branch worktree.
luau test.luau->ok.noctalia plugins lint timew->0 errors, 0 warnings. Nogetfenv/setfenvintimew/. EveryonClickis still a name off the model, no closures.git status --porcelainempty in both worktrees; everything I broke was broken in
/tmp/pr10/workand/tmp/pr10/h, and each restore wascmp-checked.Not checked
runAsync's realscheduling, the real
noctalia.state.watchreplay semantics and the realwidget's handling of
focus/keyare inferred. F3/F4 infocus.luauare myattempt to bracket the two that matter, but they are models of a host, not the
host.
day 99. Reproduced identically onboth sides (
R6: 0 paints on the click,dayTitle--on the next tick), notinvestigated.
Parse/Query/Timew/the region builders. Waves 1-5 territory; I onlyconfirmed
viewModelandrenderreadstateand write nothing.poke/watchingstaying outside the table is right beyond confirmingneither appears in
viewModel.Panel states worth screenshotting
The two that changed, and three the harness can only assert about:
drops a paint and a 200-id walk. Watch for a single settle rather than the
chip appearing over the old list first.
day-scope list for one frame; there should now be no such frame.
the tag field exactly once, and the adopted interval must be on that first
tree -- this is
onOpen'sapplyandadoptState'sapplyboth cashing inon the same
commit.timew. The selection, the scope switch and thepager rewind are applied without a tree until the day's intervals land. Same
as main, but it is the longest-lived apply-without-paint window in the file
and the only one a human can catch.
state.shownpublication finding 1 is about; on a correct build the chip doesnot move.
All four applied,
cd3395b. Both P2s were the review's own diagnosis reproducedfirst and fixed second; nothing declined. The panel change is comments only —
git diffontimew/panel.luauhas no non-comment line either side.1. The bypass invariant (applied)
Reproduced your mutant exactly before touching anything, so the "before" is mine
and not inherited:
Your
paintsForsuggestion applied as written. After, on the same mutant, atcd3395b:Restored,
cmp-clean, pristine back took. 23 counted paths, 23 bypass checks.On the claim: the section comment now separates the three things by who proves
them. The count proves mutate-without-painting.
paintsFor's new block provespublishes-what-it-painted. The source shape stays grep-only in the gate, and
the comment says so and says why (
:743, same reason en.json is a copy) insteadof saying "nothing bypasses
commit".Your extra hole is written on the
statetable itself, with the grep, since youare right that the whole design rests on that grep:
Re-verified on the branch: no matches.
2. The globals snapshot (applied)
Confirmed dead before fixing, on the branch:
Moved to immediately after the
panelEnv.panelstub withenvBefore.os = trueseeded, old block deleted. Same mutant, at
cd3395b(panel.luau:1111now):One check you did not ask for, because a hand-seeded key is exactly the kind of
thing that quietly becomes a blanket exemption: the seed is load-bearing, and
deleting it fails on a pristine panel.
That is what pins the snapshot as genuinely early rather than merely moved, so
it is written into the comment as the reason the seed is there.
The half you told me to keep is kept and re-verified live. Planted at
viewModel's tail rather than its head, so the current call's tree is stillcorrect and only a later call sees the wipe:
Note which assert catches it: the named-values one, not the dump comparison —
the dump is taken after
open()has already run the model once, so the wipe isalready idempotent by then. Your "idempotence is not purity" reasoning is what
makes that half work at all; the dump alone would pass.
3.
adoptStatenever measured (applied, and further than asked)Both your options were "measure it or admit it is argued". I measured it, with
your
adopt.luau—+1 paint per open, +1 per mutating command, first treeidentical either way — and the PR body now carries that block instead of leaning
on the repaint table.
But the harness gap was the smaller half of it.
noctalia.state.getreturnsnilin test.luau too, soadoptState's success path was not merelyunmeasured, it was the one panel path the suite never executed — while the
section comment claimed "every path the panel has is counted here". Same defect
class as findings 1 and 2, so it is closed the same way rather than annotated:
the path is now driven with a publishing bar widget, asserting that adopting
costs no tree of its own, that the adopted interval is on
onOpen's single tree(action button
onStop, subject line the adopted tag, both read before any fetchhas landed), and that the four landings still cost one paint each.
Mutation-proved both ways, at
cd3395b:So the site is justified by measurement in
/tmp, and now defended by an assertin the repo. Sixth
applycounted; the "every path" claim is true.4. The awk output is two lines off (applied)
It was, and it is worse than two lines now, because my own fixup moved them
again — the panel comment added ten lines above the block:
Description re-quoted at
345 / 367 / 373. Same three sites, same conclusion.The mutation-proof table had the same rot at eleven rows' scale after my commit
shifted
test.luauby ~104 lines. Rather than re-quote numbers that will moveon the next edit, that table is now anchored — "line numbers are at
84d4201",which
git checkout 84d4201reproduces verbatim — with the assert message asthe stable identifier, and the fixup's six mutations in their own table at
cd3395b. The two references outside it (:636/:640→:659/:663, focusone-shot
:1060→:1070) are rebased.Also written down
Your
pairs(patch)note, onapply's comment, since it is the one semanticnarrowing the rewrite makes and nothing tests it:
Verified that last clause rather than asserting it:
panel.luau:706ispatch.startEpoch, patch.tags = 0, "".Gate
Tree identity and the repaint table both re-run against the fixup's
timew/panel.luau(harnesspbranch.luaurefreshed,cmp-identical to theworktree copy). Neither moved. Every mutation above was run in
/tmp/pr10fix,restored with
cpand confirmed withcmp, and the suite is green on thepristine tree before and after each one.
Screenshot list extended with your items 5 and 3 — the filter-then-click state
finding 1 is about, and the idle open with the bar widget publishing. Merging.