PR #404 follow-ups: expect_row_label arg grammar, malformed-shape guard, seed docs nf-phit ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/410 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #404.

All below the gate on that PR — it is CONVERGED. Filed so the thread can close.

1. expect_row_label doesn't follow the arg grammar its siblings settled on (#351). expect_hint and expect_bar give "" and "*" explicit meanings (shared/VisualProbe.cs:596-605). expect_row_label is plain exact-equality against LabelShown, whose initial value is also "" — so {"expect_row_label": [n, ""]} passes both for "the label drew nothing" and for "the label exists but _Draw never ran". No probe uses it today; it is a trap for the next author, and it is the same class #351 closed for expect_hint.

2. The malformed-arg guard covers wrong arity, not wrong shape. erl.EnumerateArray() throws InvalidOperationException on {"expect_row_label": "SILKEN"}, and e.GetInt32() throws on ["SILKEN", 4] — neither reaches the [probe] expect_row_label wants [rowIdx, "TEXT"] line the branch exists to print. Consistent with expect_pixel's idiom, so not a regression, but the branch comment claims a loudness the code delivers only for the arity case. A ValueKind check ahead of the loop covers both, and would fix expect_pixel at the same time.

3. The probe seed's blast radius is under-documented. games/tonapse/TonapseHome.cs says "Change it and tools/probes/row-label-wrap.json must change with it", but EditorState.NameRng is process-global under *every* probe run — any future probe that asserts a generated name is downstream of seed 4 too. (Checked at review time: grid-scroll.json is the only other script that presses + new, and it asserts focus only.) One clause in the comment.

4. No unit test pins the NameRng threading. games/tonapse/EditorState.cs is cross-linked into the test project and games/tonapse/tests/WaveletNamesTests.cs already exists. NameRng = new Random(4)NewSound() → assert silken-chestnut would gate the seed contract without a Godot run, and would fail as a named test on a future .NET Random algorithm change rather than as a mystery probe RED.

5. tools/probes/row-label-wrap.json asserts rows 0, 1 and 3, skipping row 2 (drums). No reason given, and drums is a single-line row like its neighbours. One line closes the set.

Refs #404, #366

Dependencies

None.

Comments

No comments.

Add a comment