Reshape the Guppy DEM construction API into the house builder pattern

[ciaranra]

Problem

build_dem_from_guppy(...) is a verb-named function that takes every option as
a keyword argument and returns a GuppyDemBuild noun you then unpack:

dem_build = build_dem_from_guppy(
    rep_code_memory,
    num_qubits=7,
    detectors=detectors,
    observables=observables,
    idle_after_2q_duration=1.0,
    p_idle_linear=0.01,
    p_idle_linear_model={"X": 0.25, "Y": 0.25, "Z": 0.5},
    p_idle_sin_squared=0.03,
    p_idle_sin_squared_model={"Z": 1.0},
    p1=0.002,
    p2=0.02,
    p_meas=0.02,
    p_prep=0.02,
)
dem = dem_build.dem

This reads as an inverted builder: the configuration is a flat keyword pile
(now ~30 parameters wide after the structured idle families), the call site
cannot be composed incrementally, and the result object is the thing you would
normally get from .build(). It also diverges from the house builder-of-builders
style used by sim(...)/sim_neo(...), where configuration is chained and a
terminal call produces the artifact.

Proposal (sketch, to be settled in review)

dem_build = (
    dem_from_guppy(rep_code_memory)
    .qubits(7)
    .detectors(detectors)
    .observables(observables)
    .idle_after_2q(1.0)
    .noise(gate_noise(p1=0.002, p2=0.02, p_meas=0.02, p_prep=0.02))
    .idle_noise(linear(0.01, {"X": 0.25, "Y": 0.25, "Z": 0.5}), sin_squared(0.03, {"Z": 1.0}))
    .build()
)
dem = dem_build.dem

Points to settle:

• Whether the noise configuration groups into its own builder(s) shared with the
  other consumers (ties into Port the structured idle-noise families to the native-surface NoiseModel (twirl/threshold route) #422, Deprecation ramp: align pecos-engines GeneralNoiseModel idle-noise API with the structured vocabulary #424, Align the non-idle GeneralNoiseModel surfaces (gate/emission/leakage/measurement/crosstalk) with the structured noise vocabulary #425 and the layer-2 channel vocabulary
  in Layer-2 typed idle-channel vocabulary and EEG coherent-route alignment #423 — a channel list is a natural fit for a chained .idle_noise(...)).
• Whether DetectorErrorModel.from_guppy (the JSON-metadata entry point) gains
  the same shape or stays the one-call convenience form.
• Migration: both entry points are public and documented; a deprecation ramp is
  needed rather than a rename, as in Deprecation ramp: align pecos-engines GeneralNoiseModel idle-noise API with the structured vocabulary #424.

Related

Same consistency theme as #422-#425. Non-blocking for PR #420, which uses the
current shape; the docs there name the result dem_build rather than build so
the verb/noun collision at least does not appear in the examples.

---

Unify the two Guppy DEM entry points (added after review of the current surfaces)

There are two entry points that do the same job:

	DetectorErrorModel.from_guppy	build_dem_from_guppy
detectors/observables	JSON strings	typed Detector/Observable (bare tag strings, rec[-k], result_ref)
returns	the DEM	GuppyDemBuild: .dem, .audit, .schema_fingerprint, .evaluate_result_columns()
pipeline	trace, normalize, idle passes, attach metadata, Rust builder	identical

Measured overlap: 38 and 37 parameters, 35 shared. The only differences are
detectors_json/observables_json/num_measurements versus
detectors/observables. They produce byte-identical DEMs for the same
specification (verified while writing docs/workflows/guppy-dem-decoding.md).

The duplication is not free: every parameter added must be added twice, in two
signatures, two docstrings, and two guard wirings. PR #420 did exactly that four
times (idle passes, the three idle-noise families, the symmetric-default and
L-key round, and the p_idle removal).

Direction

One construction pipeline, two convenience shapes at the surface:

• The builder from this issue is the single pipeline and returns the rich
  object (.dem, .audit, .evaluate_result_columns()).
• DetectorErrorModel.from_guppy stays as the one-call convenience and keeps
  returning the DEM (changing its return type would be breaking), but delegates
  to the builder and accepts either spelling for detectors/observables:
  typed specs or the JSON strings that other tooling emits (e.g.
  meta_tc.get_meta("detectors") from the surface builder).
• build_dem_from_guppy becomes a deprecated alias for the builder path,
  retired on the ramp described above.

Net effect: parameters live in exactly one place, the typed and JSON spellings
stop being a fork, and the audit trail plus result-column evaluator become
reachable from the documented entry point rather than only from the second one.

[ciaranra]

Design review panel (2026-08-03, two blind arms) broke the plan in this issue. Design record with all findings: pecos-docs/design/construction-and-decoding-api-shape.md.

Key corrections, all verified against the code:

1. The dual-spelling idea is withdrawn. rec[-k] and {"records": [-k]} are different coordinate systems: typed RecordRef resolves to a canonical dense MeasId and is emitted as meas_ids (dem_spec.py:424-435, :517), while JSON records is positional in the runtime record (builder.rs:697-713, :3597-3607). On the repo's own reorder fixture, typed rec[-3], rec[-1] gives MeasIds [0, 2] and JSON records [-3, -1] gives [2, 1]. Accepting both on one parameter would silently pick a coordinate system.
2. The byte-identity justification was overstated. The two entry points agree for tag references (verified); for offset references "the same specification" is not well-defined, and no cross-entry-point equality test exists in the repo. That test — under a reordering runtime — is prerequisite work, and it should fail today.
3. Neither spelling is a superset. Typed cannot express runtime-positional records at all; num_measurements exists only on the JSON side. The paths also apply different validation (typed enforces dense detector ids, XOR-cancellation, zero-parity and reserved-key rejection; JSON defers everything to Rust).
4. The dual-spelling motivation is already solved: surface_memory_dem_spec(...) converts the surface builder's JSON metadata into typed specs (dem_spec.py:147-204).
5. The parameter-count complaint is mostly a noise-vocabulary problem. Collapsing 22 of the 35 shared parameters into one structured argument (Port the structured idle-noise families to the native-surface NoiseModel (twirl/threshold route) #422-Align the non-idle GeneralNoiseModel surfaces (gate/emission/leakage/measurement/crosstalk) with the structured noise vocabulary #425) largely dissolves "38 vs 37" with no shape change. That vocabulary spans seven signatures plus the NoiseModel dataclass — which already has the structured shape being proposed and should be the model rather than a migration target.
6. Missing surfaces: from_circuit, DemSampler.from_circuit, DemBuilder, and DemSamplerBuilder were all omitted. DemSamplerBuilder is already a chained builder, so construction should adopt its conventions rather than invent a second set. Also worth noting: from_guppy is a lossy subset of from_circuit (no idle_rz, no measurement-crosstalk parameters, no per-gate rate maps).

Revised order (the original was backwards): settle rec/records + add the missing test → decoder defects #430/#431 → noise vocabulary #422-#425 → builder (#428) → decoder protocol (#429). Deprecating build_dem_from_guppy before step 1 would point 36 call sites at a records that means something else — silent wrong DEMs, not errors.

Migration facts for whoever picks this up: 33 from_guppy call sites, all using detectors_json=; 36 build_dem_from_guppy call sites; two tests construct GuppyDemBuild by keyword; no isinstance checks anywhere, so a return-type change fails at attribute access rather than loudly.

[ciaranra]

Correction (2026-08-03): the blocker in my earlier comment does not hold.

That comment withdrew the dual-spelling idea on the grounds that rec[-k] and {"records": [-k]} are different coordinate systems that diverge under measurement reordering. Both review arms reported it; neither had executed it for the JSON side. I have now tested it directly, using the builder's own redundancy rule as the oracle (co-present records and meas_ids must resolve to the same set, so an accepted pair proves the two spellings name the same measurement):

• Traced Guppy path, 5 measurements: records[-5] is accepted only against meas_ids [0], rejected against 1-4.
• Hand-built circuit with stamped ids [2,0,1] (execution order genuinely non-canonical): records[-3] is likewise accepted only against meas_ids [0].

The two spellings agree on every fixture available in the repo. The derivation missed that meas_ids resolution falls back to identity when the influence map carries no stamped ids (builder.rs:605-613), which is the actual behavior on these paths.

The divergence remains possible in principle — records is an absolute index (types.rs:1894-1900) while meas_ids is a position lookup, so a runtime that reorders measurements relative to canonical order would separate them — but no runtime in the repo produces that condition, so it cannot be demonstrated either way today.

python/quantum-pecos/tests/qec/test_record_vs_meas_id_semantics.py (commit 321b7e04d) pins the current agreement, including a negative case so it cannot pass vacuously. If a reordering runtime ever appears, that test fails loudly rather than detectors silently rebinding.

What this changes for this issue: the dual-spelling parameter is no longer ruled out on correctness grounds. The remaining arguments against it stand on their own and are weaker: neither spelling is a superset (typed cannot express positional records; num_measurements is JSON-only), the two paths apply different validation, and surface_memory_dem_spec already provides the JSON-to-typed conversion the escape hatch was meant to serve. Those are design-taste arguments, not a correctness boundary — so the unified entry point is back on the table and should be decided on its merits.

The design record pecos-docs/design/construction-and-decoding-api-shape.md has been corrected accordingly, including a note that agreement between two independent review arms raised confidence without establishing truth.

[ciaranra]

Unified builder landed in PR #420 (commit 51bef50d5).

build = (
    DetectorErrorModel.builder()
    .program(program)
    .qubits(7)
    .detectors(detectors)
    .noise(noise_model)
    .idle_after_2q(1.0)
    .build()
)
dem = build.dem

The unification is the delegation, not the syntax. DetectorErrorModel.from_guppy and build_dem_from_guppy are now thin wrappers that translate their arguments into chained setters and select .dem versus the full GuppyDemBuild. There is one tracing, noise, idle-pass, schema, DEM-construction, and audit pipeline, in GuppyDemBuilder.build(). Both functions keep their exact signatures and defaults; their existing tests passed unchanged as the oracle for that.

Design decisions, with reasoning recorded in pecos-docs/design/construction-and-decoding-api-shape.md:

• Entry is DetectorErrorModel.builder(), no-argument, with the program supplied by .program(...) like every other input — nothing special-cased into the constructor.
• Not on DemBuilder. That name is taken, and DemBuilder is the low-level builder over a fault influence map whose own docstring directs users to from_circuit(). A "trace a Guppy program" front door would invert its layer, so this unifies style and pipeline rather than class.
• Bare setters, not with_*. Measured across this repo: with_* appears 17 times (all on low-level builders) versus 33 bare setters (all five decoder builders, plus sim()/sim_neo()). The split tracks user-facing versus low-level, not old versus new. Rust does not settle this either — the API Guidelines leave builder naming explicitly open and reserve with_ for constructors (Vec::with_capacity); crates split between bare setters for scalar config (tokio, reqwest, clap) and with_* for attachment (tracing-subscriber). This is scalar config on a Python surface.
• The return-type fork dissolves: .build() returns the existing GuppyDemBuild, so the rich object is the terminal and .dem is one attribute away. No new result type, no proxying magic.
• Setter misuse fails loud: duplicate setter calls, mixing typed and JSON spellings for one role, num_measurements with typed specs, and missing program/qubits/detectors at .build().

One thing I changed during review: the first implementation smuggled the legacy flat keywords through the public .noise() setter as a magic 3-tuple carrying dict(locals()). That made a publicly-typed setter silently polymorphic and captured every local at the call site. .noise() is now strictly a NoiseModel setter (verified: a tuple raises TypeError), and the legacy path goes through a private _legacy_noise seam with an explicitly-built keyword dict.

No deprecation yet — the two functions have ~69 call sites between them and a ramp deserves its own change.

Verification: full tests/qec suite 1381 passed, 1 skipped, 1 xfailed; generated doc tests for both affected pages pass; repo-wide lint clean on a cold run with .ruff_cache cleared.