P6.x: sweep emitter/wrangler.rs off bynk_syntax::ast (slice 2 of #1187)

[accuser]

Summary

• emit_wrangler_toml's two raw matches on a handler's cron kind and a service's queue-binding
  protocol relocate to its one call site in project.rs, which already imports bynk_syntax::ast
  and is already counted by the ast_importers probe. crons/queues now arrive as pre-collected,
  sorted+deduped &[String] parameters instead of being walked here off table.services — that was
  this file's entire raw-syntax footprint, so relocating it drops wrangler.rs out of the probe
  outright.
• No bynk-emit::ir equivalent exists to route through instead: no project-wide IrItem::Service is
  built at the call site, and IrHandler::kind reuses HandlerKind unchanged even where one is, so an
  IR-based version would still have matched the cron kind directly.
• emitter/runtime_use.rs, originally paired with wrangler.rs in P6.x candidate: cutover slicing — emitter.rs/emitter/lower.rs from bynk_syntax::ast to bynk-emit::ir #1187's own table as a joint
  trivial sweep, is deliberately untouched here — corrected in P6.x: sweep emitter/wrangler.rs off bynk_syntax::ast (slice 2, narrowed — runtime_use.rs deferred) #1191 and via a comment on P6.x candidate: cutover slicing — emitter.rs/emitter/lower.rs from bynk_syntax::ast to bynk-emit::ir #1187. Its
  TypeRef field is downstream of emitter/serialisation.rs's still-AST-driven JSON-codec renderer
  (serialise_expr/deserialise_expr/ts_type_ref_qualified), a real, separate, unscoped
  conversion, not a relocation like this one.
• ast_importers moves 9 → 8 (design/greenfield-status.md regenerated). No author-facing behaviour
  change.

Test plan

• cargo build --workspace — clean
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean
• cargo test --workspace — all crates passing, including bynkc's fixture/e2e suite (cron
  fixtures 146-150/154/147, queue fixtures 393/372 — byte-identical wrangler.toml
  output)
• cargo xtask greenfield-status — ast_importers reads 8 (was 9); table regenerated with
  --apply
• cargo xtask check-pending — 1 pending file valid

Closes #1191. Part of #1137; realises slice 2 (narrowed to wrangler.rs only) of #1187.

🤖 Generated with Claude Code

[claude]

The relocation turns a self-contained invariant into an unenforced cross-file precondition, and the comment records what it is but not why it's load-bearing.

table.services is a HashMap<String, ServiceDecl> (bynk-check/src/symbols.rs:306), so iteration order is not stable across runs — the sort() is what makes wrangler.toml reproducible, not just tidy. While it lived here it was structurally impossible to lose; now a caller that forgets it produces a nondeterministic config with no compile error.

Worth spelling that out so the next person doesn't read "sorted+deduped" as cosmetic:

Suggested change

	// v0.10a: every `on cron "expr"` schedule in the context, sorted+deduped.
	crons: &[String],
	// v0.10b/v0.44: every `from queue("name")` service's bound queue name,
	// sorted+deduped.
	queues: &[String],
	// v0.10a: every `on cron "expr"` schedule in the context, sorted+deduped.
	// Sorting is load-bearing, not cosmetic: the caller walks `table.services`,
	// a `HashMap`, so unsorted input makes `wrangler.toml` non-reproducible.
	crons: &[String],
	// v0.10b/v0.44: every `from queue("name")` service's bound queue name,
	// sorted+deduped (same reproducibility requirement as `crons`).
	queues: &[String],

Also note this leaves three adjacent string-slice parameters (consumes, crons, queues) — any future transposition at the call site type-checks silently.

[claude]

Test coverage: the sort/dedup that just crossed a file boundary is not discriminatingly exercised by any fixture, so a regression here would land silently.

Checked the fixtures the PR description cites:

• 148_cron_multiple — both crons live in one service's handlers Vec (ordered), so it can't detect a dropped crons.sort() at all; and "*/5 * * * *" < "0 0 * * *" already, so declaration order equals sorted order.
• 153_queue_multiple — high-priority/low-priority come from two HashMap entries, so it would catch a dropped queues.sort(), but only as an intermittent failure, and the names are alphabetical by declaration anyway.
• 154_full_jobs, 146, 147, 149, 150, 393, 372 — single cron / single queue each.
• Nothing covers dedup() in either direction: no fixture has the same cron expression on two services in one context. (negative/118_queue_duplicate_consumer rejects duplicate queue names upstream, so queues.dedup() looks purely defensive — fine, but say so rather than leaving it as apparent live logic.)

This gap predates the PR — while the code sat inside emit_wrangler_toml it couldn't be wrong. It becomes worth closing now precisely because the invariant is a caller obligation. A cheap fix is one fixture with two services in one context sharing a cron expression and declared in reverse-sorted order — that pins sort and dedup for crons in a single expected wrangler.toml. A direct unit test on emit_wrangler_toml would work too, though it'd assert the contract from the wrong side of it.

[claude]

Review — sound relocation, two follow-ups

Verified the core claim directly: bynk-emit/src/emitter/wrangler.rs no longer contains the literal bynk_syntax::ast, and ast_importers matches on file contents (xtask/src/greenfield_status.rs:1347) — so the gated 9 → 8 move in design/greenfield-status.md is real, not asserted. emit_wrangler_toml genuinely has one call site (project.rs:2200); bynk/src/deploy/config.rs only mentions it in prose. HandlerKind/ServiceProtocol were already in scope in project.rs. The relocated loop is substance-identical — same table.services walk, same match shapes, same sort-then-dedup ordering. Vec<&String> → Vec<String> does not change sort or dedup semantics (Ord/PartialEq on &String delegate to String), so byte-identical output is the expected result. Also confirmed no test pins a hardcoded survivor list that would need updating alongside the count.

Findings

1. The precondition is now unenforced, and the comment does not say why it matters (inline on wrangler.rs). table.services is a HashMap (bynk-check/src/symbols.rs:306), so sort() is what makes wrangler.toml reproducible. Inside the function that was structurally guaranteed; as a caller obligation it is a silent failure mode. Suggested a two-line comment. Minor related point: consumes, crons, queues are now three adjacent string-slice parameters, and a transposition would type-check.

2. The moved edge is not discriminatingly tested (inline on project.rs, with the per-fixture breakdown). In 148_cron_multiple both crons live inside a single service and its ordered handlers Vec, so it cannot detect a dropped cron sort; 153_queue_multiple would catch a dropped queue sort only intermittently; no fixture covers dedup() at all. Pre-existing, but this PR is the moment it starts to matter. One fixture with two services in a context sharing a cron expression, declared in reverse-sorted order, would pin both.

Release discipline

Correct: exactly one increment, design/pending/p6-wrangler-cutover-slice-2.md, with level: patch and a changelog. patch is right for an internal relocation with no author-facing behaviour change, and no version or ADR number is picked. The design/tracks/the-ir.md update and the runtime_use.rs deferral rationale are consistent with the code.

One observation, not a blocker

The regenerated table also moves the non-gated trend rows: keep_in_sync 185 → 193 and test_density for bynk-emit (19.5% → 20.8%) and xtask (34.2% → 34.8%). Neither is attributable to this diff — keep_in_sync counts comments containing "in sync"/"mirrors"/"parity"/"must match" and this PR adds none, and the PR adds only non-test lines to bynk-emit, which would push density down, not up. So these look like drift from earlier merges swept up by --apply. Harmless, since only ast_importers is gated, but the numbers are not evidence about this change and are worth not reading as such.

Neither finding blocks the merge — the relocation itself is faithful.

[accuser]

Both findings addressed in 68bc1f1:

1. Why sort is load-bearing — wrangler.rs:56-60 now spells out that table.services is a
   HashMap, so the sort is what makes wrangler.toml reproducible across runs, not cosmetic (your
   suggested wording, essentially verbatim).

2. Test coverage — added 1191_cron_sort_dedup_across_services: two services in one context with
   distinct cron schedules declared in reverse-sorted source order. Verified by hand (temporarily
   commenting out crons.sort()) that this fails intermittently, the same probabilistic-but-real
   coverage 153_queue_multiple already gives queues.sort() — HashMap iteration order isn't fixed,
   so it can't be made deterministic without changing what's under test.

One correction on the suggested shape: "two services sharing a cron expression" isn't constructible
as valid Bynk source. bynk.cron.duplicate_schedule (context_checks.rs:1830-1856) already rejects
a duplicate cron schedule across all services in a context — same pattern as
bynk.queue.duplicate_consumer for queue names, which is presumably why queues.dedup() reads as
defensive to begin with. So crons.dedup() is equally unreachable from any positive fixture, for the
same reason, not just currently untested. The added fixture covers the sort with two distinct
schedules instead; dedup stays untested by design, matching queues.dedup()'s existing status quo
rather than fixing an asymmetry that doesn't actually exist.