Skip to content

test(sampler): pin the parameter SET, not a bare count#6441

Merged
shipyard-local[bot] merged 1 commit into
mainfrom
fix/sampler-param-contract
Jul 22, 2026
Merged

test(sampler): pin the parameter SET, not a bare count#6441
shipyard-local[bot] merged 1 commit into
mainfrom
fix/sampler-param-contract

Conversation

@shipyard-local

@shipyard-local shipyard-local Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

main is red

Any lane that builds examples currently fails. PulpSampler registers ten parameters; the contract test still asserts nine.

registered ids: 1, 2, 3, 4, 5, 6, 7, 8, 9, 10
declared ids:   1, 2, 3, 4, 5, 6, 7, 8, 9

The tenth is kSamplerHeritageClockRatio, added with the sampler heritage kit (39c7dac, #6396).

Which side is wrong: the test

The parameter is real and fully wired, so it is not the thing to remove:

  • declared — pulp_sampler.hpp:46, kSamplerHeritageClockRatio = 10
  • readpulp_sampler_render.cpp:94, state().get_value(kSamplerHeritageClockRatio)
  • exercised — test_pulp_sampler_heritage_stream_domain.cpp:505/525/541 drive it at 2.0 and 4.0
  • fully specified — "Heritage Clock Ratio", range 0.25–4.0, default 1.0, step 0.01

#6396 did edit this contract file — it updated the resource-estimate assertion and missed the count one. So this is a stale assertion, not a rogue parameter.

Why assert the set instead of bumping 9 → 10

A bare count fails as 10 != 9, which names nothing. Re-reading nine as ten would leave the identical trap for the eleventh parameter. Asserting the ordered set makes the failure self-describing — the two INFO lines above are the actual output of the negative control.

Why nothing caught it

The required PR gate builds with PULP_BUILD_EXAMPLES=OFF, so no PR or merge_group check compiles this file. nightly-full-build.yml does build examples ("Deliberately NO -DPULP_BUILD_EXAMPLES=OFF"), so the lane that catches it runs only after landing — and release-cli builds examples too, which is where a stale example break surfaces if nobody is watching nightly.

That gate hole is worth addressing separately. Adding examples to the PR gate is the wrong fix — they are the heavy targets, which is why they are off. The candidates are making the nightly failure reach someone, or building examples in merge_group. The latter needs care: merge_group is serial at max_entries_to_build: 1 and its check_response_timeout is already shorter than a legitimate validation.

Verification

  • pulp-sampler-test, full binary, with this change: 4613 assertions in 186 test cases, all passed
  • Negative control: omit the tenth id from the declared set → fails, printing the two lines above, then passes again when restored
  • tools/scripts/gates.sh origin/main: green

Found by the VST3 hosted-editor session, which deliberately did not fix it to avoid smuggling a sampler change into a VST3 PR. They left open whether the test or the parameter was wrong; the wiring evidence above answers it.


🔎 Provenance

Agent claude
Machine m3
Tab Set up fleet v3 project with Orchestrate in new workspace
Directory /Volumes/Workshop/Code/pulp-mq
Session 1dd2217d-ca26-4de4-b90e-6360ee343606

Resume

claude --resume 1dd2217d-ca26-4de4-b90e-6360ee343606

Jump to this tab

cmux surface focus 22A04F4D-188B-4306-91D2-ECAEADCC7F13

Relaunch (any agent)

cmux surface resume get --surface 22A04F4D-188B-4306-91D2-ECAEADCC7F13

Restore URLhttps://claude.ai/code/session_01363VPyLiV9Er9DJ1pcmC9c

stamped 2026-07-22 00:27 UTC

`main` is red for any lane that builds examples. PulpSampler registers ten
parameters, and the contract test still asserts nine:

    registered ids: 1, 2, 3, 4, 5, 6, 7, 8, 9, 10
    declared ids:   1, 2, 3, 4, 5, 6, 7, 8, 9

The tenth is `kSamplerHeritageClockRatio`, added with the sampler heritage kit.
It is a real, wired parameter — declared in the header, read by
`pulp_sampler_render.cpp` through `state().get_value()`, exercised by the
heritage stream-domain tests, and fully specified (0.25–4.0, default 1.0). So
the parameter is right and the assertion was stale: the commit that added it
updated this file's resource-estimate assertion and missed the count.

Assert the ordered parameter set instead. A bare count fails as "10 != 9", which
says nothing about which parameter moved, and re-reading nine as ten would leave
the same trap for the eleventh. The set names the drift on the INFO lines above.

This was invisible on the way in: the required PR gate builds with
PULP_BUILD_EXAMPLES=OFF, so no PR or merge_group check compiles this file.
`nightly-full-build.yml` does build examples, so the lane that catches it runs
only after landing — and release-cli builds examples too, which is where a stale
example break would otherwise surface.
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 21, 2026
Merged via the queue into main with commit 68594c7 Jul 22, 2026
17 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant