Skip to content

Serve the examples by path so their shipped data comes with them - #116

Merged
max-sixty merged 5 commits into
mainfrom
fix/ci-33274587967
Aug 29, 2026
Merged

Serve the examples by path so their shipped data comes with them#116
max-sixty merged 5 commits into
mainfrom
fix/ci-33274587967

Conversation

@leaf-agent

@leaf-agent leaf-agent commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Pass example paths to serve at the four call sites that previously passed copied markup, and set seed_log=False where the shipped conversation is outside the test. The fixture now installs the external data selected by each example without changing the page state those tests exercise.

Update the test guidance to preserve the same distinction: an example path carries its external data and event log, while seed_log=False omits only the conversation.

Testing: the affected gate, gutter, and ring cases pass together (8 passed), and pre-commit passes on all four changed files.

This was written by Codex on behalf of max-sixty

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the fix from the merged tree: with the three test files reverted to fdaa3ea, test_a_reload_mid_flight_never_wedges_round_trip and test_the_gutter_runs_beside_the_exhibit_and_no_further[gallery]/[ship-review] fail on exactly the activate_source assertion the description quotes; with the PR's versions, those plus both ring readings pass (8 passed, --run-nightly). seed_log=False holds the prior behaviour where it matters — ship-review.jsonl is the only seed among the four examples touched, and the string call sites never laid it in either — so the only change to what is served is the data, which is the point.

One thing the PR doesn't reach, and it's the durable half of this bug. tests/CLAUDE.md still tells the next author to spring the same trap:

Handed an example's path rather than its markup it also lays in the log that example ships … Pass the markup where the log would be noise for the subject, and say which in a comment.

(tests/CLAUDE.md, the serve paragraph — search for "Pass the markup where the log would be noise".) Two things are now wrong there. The paragraph names only the log and the media as what a path lays in, where the fixture's own docstring names three — "the media it names, external data, and the event log". And the instruction that follows is the idiom this PR is removing from the suite: a test whose subject isn't the conversation is told to pass markup, which drops the data silently rather than the log deliberately. The description's closing claim — "after this no test hands serve an example's markup as a string" — is true of the suite (I checked; COMMAND_HUB_PAGE and the _publish call sites are v2 markup, not serve arguments), but it isn't true of the guidance, so the corpus regrows the call site the moment someone follows it.

The silent case is the one worth guarding: this run failed loudly only because gallery and ship-review select a $captures snapshot, so activate_source refused. An example shipping only replaceable set values in its .data.json would have served without them and gone green over a page the reader never gets.

The fix is the sentence, not the tests: name external data alongside the log, and point at serve(example, seed_log=False) as the way to leave the conversation off — which is what this PR, and test_render_pages.py before it, actually do. It is a one-sentence change and it belongs with these tests rather than in a PR of its own.

@leaf-agent leaf-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guidance fix landed and it is the right one. Checked the PR's version of the paragraph with git show HEAD:tests/CLAUDE.md (the PR run restores base copies of CLAUDE.md paths into the worktree, so the checkout reads stale here): it now names the external data alongside the log as what a path lays in, and points at serve(example, seed_log=False) instead of telling the next author to pass markup.

The seed_log=False claim holds in the fixture — in render_harness.py's go, the data_operations(example) loop is guarded by if example: alone, while the seed append and the cursor.json write are the only things seed_log gates. So the sentence's promise that "the external data still belongs to the example" is what the code does, and the silent-drop path the earlier review worried about no longer has an instruction pointing at it.

Nothing further from me. Self-authored, so no approval from this account.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

ci on main failed again with this PR's failure shape — run 33276669997 on f9a66b7 ("Make the comment-button clearance test able to fail again (#103)"). All three failures are ones this PR fixes, and nothing else in the nightly suite failed (3 failed, 1499 passed, 6 skipped).

FAILED tests/test_render_gate.py::test_a_reload_mid_flight_never_wedges_round_trip - AssertionError: index.html <lf-source> input `document` (line 233) selects snapshot '1' from source 'offline-review-policy', but data.json does not contain it
FAILED tests/test_render_options.py::test_the_gutter_runs_beside_the_exhibit_and_no_further[gallery] - AssertionError: index.html <lf-source> input `document` (line 233) selects snapshot '1' from source 'offline-review-policy', but data.json does not contain it
FAILED tests/test_render_options.py::test_the_gutter_runs_beside_the_exhibit_and_no_further[ship-review] - AssertionError: index.html <lf-source> input `document` (line 230) selects snapshot '1' from source 'offline-review-policy', but data.json does not contain it

The two extra failures from the run this PR was opened fortest_example_renders[gallery] and [parallel-workstreams], both ['[light] <ma...s reaches it'] — did not recur here, and neither did test_every_ring_the_layer_draws_is_shown_whole_somewhere_in_the_corpus. So the data.json failures are the deterministic remainder, and main stays red until this lands.

No new PR opened, since this one already covers the failures.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

This PR also covers the whole remainder of run 33275975630 (7df11d1), which tend-ci-fix fired on separately: its six failures are the three data.json ones above plus test_example_renders[gallery]/[parallel-workstreams] and test_every_ring_the_layer_draws_is_shown_whole_somewhere_in_the_corpus, and those last three now pass on mainfd92a11 put :where() on the tabs frame rule and #114 took the margin preview's room from the page's box. So no separate fix is owed for that run.

New, and not covered here: two gate cases went red at fdaa3ea ("Unify right-margin actions"), so this PR's own test job should fail on them.

FAILED tests/test_render_gate.py::test_a_page_hands_its_note_strip_back_when_the_panel_takes_the_room - AssertionError: content set outside a column the strip had crushed: ['<lf-option id=nx-revoke> is drawn 14px outside <lf-options id=next-jobs>, which does not show it', ...]
FAILED tests/test_render_gate.py::test_a_change_may_be_decided_over_the_note_it_stands_level_with - playwright._impl._errors.TimeoutError: Page.wait_for_function: Timeout 30000ms exceeded.

Both are deterministic and bisect to that commit, with no working-tree changes: uv run pytest <the two> --run-nightly -n0 is 2 passed at 62560cc and 2 failed at fdaa3ea. fdaa3ea's message records uv run pytest tests (706 passed), which is the suite without --run-nightly, so the browser corpus did not run against it. They are a separate concern from this PR's harness fix — ci on fdaa3ea is still in progress and will raise them on its own.

@leaf-agent

Copy link
Copy Markdown
Collaborator Author

tend-ci-fix fired again for run 33276679392 on 10460ab ("Repaint the key line when a disclosure changes, not when one is restated (#112)"). Same three data.json failures this PR fixes, plus a fourth that #115 closed four minutes after that run finished — so nothing is owed beyond this PR, and no new PR was opened.

The run's four failures and where each is answered
FAILED tests/test_render_gate.py::test_a_broken_probe_module_is_a_gate_finding - assert False
FAILED tests/test_render_gate.py::test_a_reload_mid_flight_never_wedges_round_trip - AssertionError: index.html <lf-source> input `document` (line 233) selects snapshot '1' from source 'offline-review-policy', but data.json does not contain it
FAILED tests/test_render_options.py::test_the_gutter_runs_beside_the_exhibit_and_no_further[gallery] - AssertionError: index.html <lf-source> input `document` (line 233) selects snapshot '1' from source 'offline-review-policy', but data.json does not contain it
FAILED tests/test_render_options.py::test_the_gutter_runs_beside_the_exhibit_and_no_further[ship-review] - AssertionError: index.html <lf-source> input `document` (line 230) selects snapshot '1' from source 'offline-review-policy', but data.json does not contain it

10460ab predates db7617f, so the probe-module case is #115's and is already fixed on main; the other three are this PR's.

Read both ways from this checkout: at main's db7617f the reload and [gallery] cases still fail on the same assertion (-x stopped the run there). On a worktree of this PR's head a165016, which already has db7617f merged in, all four pass — 4 passed in 24.06s.

@max-sixty
max-sixty merged commit 9973180 into main Aug 29, 2026
4 checks passed
@max-sixty
max-sixty deleted the fix/ci-33274587967 branch August 29, 2026 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants