Skip to content

Burn the optional-seam baseline in GenWave.Orchestration down to zero (SPEC F193.2) #827

Description

@genwave-radio

🎯 The rule, and why it exists

SPEC F193.2 forbids an optional seam: a constructor parameter typed as an interface that
carries a default value (IStationClock stationClock = null). It is a dependency a class can
quietly resolve for itself, which means:

  • a test can build the object graph without wiring the real collaborator, so tests and
    production disagree about what is actually plugged in;
  • adding a new collaborator breaks nothing at compile time, so call sites silently keep the
    old shape.

PLAN T536 review finding F5 stripped these off BreakDelivery for exactly this reason, and the
gh-#401 split shipped the fitness test that enforces the rule.

📍 Where we are

The rule is true for the classes the split created. It is false for 25 parameters across 10
classes
that predate it. PLAN T537 shipped a named, dated baseline —
tests/GenWave.Architecture.Tests/Support/OrchestrationConstructorSeams.cs,
KnownViolationsAsOf20260920 — asserted by Story460_TheOrchestratorAfter.NoInterfaceParameterHasADefault.

The assertion is set equality, deliberately, not a one-directional difference. So:

  • a genuinely new violation fails it, and
  • a violation fixed without striking its baseline entry in the same change also fails it.

That second direction is what makes this issue a burn-down rather than a wish: the test already
refuses to let a fix rot into a stale forgiveness.

SPEC F193.2 currently names this issue as the schedule that returns it to its absolute form.

🧮 The debt, measured

Call sites are new <Class>( occurrences across src/ + tests/, plus target-typed
new(…) construction where a helper's return type supplies the type (the text scan is blind to
those — see gh-#828).

Class Optional params Call sites Note
MusicSelectionPolicy 3 47 envelopeProvider, personaPickProvider, requestFulfillmentSource
ScheduleResolver 1 44 stationClock (42 explicit + 2 target-typed)
RollingPatterDurationEstimator 1 35 copyBounds
CachingScheduleResolver 1 25 showStore
BreakRenderer 2 21 announcementCopyWriter, announcementRenderer
PersonaRanker 1 16 stationClock (13 explicit + 3 target-typed)
HandoffCeremonyProducer 2 11 personaStore, speakerSnapshots
ClockAnchoredImagingProducer 1 11 stationClock
BreakPlanner 10 10 the whole seam set
Orchestrator 3 9 imagingSettings, observer, patterEstimator (2 explicit + 7 target-typed)
Total 25 229 across GenWave.Orchestration.Tests + GenWave.TestSupport

Every site is in test code. There is no production risk in this burn-down and no behaviour
change
— it is a compile-time-safety recovery.

🗓️ The schedule — four PRs to zero

Each PR: remove the defaults on its classes, fix every call site, strike those entries from
KnownViolationsAsOf20260920 in the same change
(the set-equality fact enforces this), full
solution green.

BD-0 — prerequisite (owned by gh-#828)

Migrate the 7 spec helpers that build an Orchestrator with target-typed new(…) onto
OrchestratorBuilder. This drops the Orchestrator's own sites from 9 → 2, so BD-1 below costs
two call-site edits instead of nine. Do it first.

BD-1 — the split epic's own classes · 16 params · ~34 sites after BD-0

Orchestrator (3) · BreakPlanner (10) · ClockAnchoredImagingProducer (1) ·
HandoffCeremonyProducer (2)

Best done while gh-#401's context is warm. BreakPlanner's 10 parameters over only 10 sites is the
cheapest ratio in the table — the single highest-value PR here.

BD-2 — the ranker, the renderer, the cache · 4 params · ~62 sites

PersonaRanker (1) · BreakRenderer (2) · CachingScheduleResolver (1)

BD-3 — the schedule pair · 2 params · ~79 sites

RollingPatterDurationEstimator (1) · ScheduleResolver (1)

Two parameters, the largest site count. Almost entirely a find-and-replace once BD-1 and BD-2 have
proven the recipe.

BD-4 — the long tail, then close it out · 3 params · 47 sites

MusicSelectionPolicy (3), then the endpoint:

  1. delete OrchestrationConstructorSeams.KnownViolationsAsOf20260920 entirely;
  2. change the fact to Assert.Empty(OrchestrationConstructorSeams.FindOptionalInterfaceParameters(…));
  3. restore SPEC F193.2 to its absolute form ("No class in GenWave.Orchestration declares an
    optional constructor parameter for a seam") and drop the baseline sentence;
  4. consider promoting the rule to a numbered CONTRIBUTING law (LawId + a CONTRIBUTING row), which
    is what T537's own remarks say it could not do while the baseline existed.

Zero exceptions reached at the end of BD-4.

🔁 The recipe, per class

  1. Delete = null (or the defaulted value) from the constructor parameter.
  2. Build. The compiler now names every call site — that is the point of the change.
  3. At each site, pass what the class was silently resolving. In tests that is usually the existing
    fake already in scope, or the shared instance the surrounding helper already built (the
    BreakDelivery change at T536 is the worked example: the helper shares one instance with the
    Orchestrator beside it rather than letting two classes resolve separate defaults).
  4. Strike the entries from KnownViolationsAsOf20260920, sorted ordinal — the list must stay sorted
    and distinct for the sequence-equal comparison.
  5. dotnet build GenWave.sln -c Release -warnaserror then the full solution.

📅 Proposed sequencing

Soft launch is 2026-10-31 and the ratified order is stability → sound → screen, so this should not
jump that queue. Proposal: BD-0 + BD-1 before launch (small, and the split's context is warm),
BD-2 → BD-4 after. Adjust as you like — the schedule is the commitment, the dates are not.

Related: SPEC F193.2 · PLAN T537 · gh-#401

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions