Share vs split¶
This page is the decision record for the duplication audit:
which of the ~300–350 lines of near-clone orchestration in the two
MainWindow code-behinds should move into Nps.Shell, which should stay
per-app on purpose, and which should never be pushed down into the engine.
"Share" here always means one owned implementation both apps call —
never two copies kept in sync by habit.
Decision criteria¶
Four questions decide every verdict below:
- Operational or domain? Operational glue (camera math, toolbar wiring, legend fill) is safe to share in the shell. Domain rules (slicing parameters, playback policy) carry product meaning and change with the product.
- How often does it change? Slow-changing code tolerates a shared owner; fast-changing code amplifies every edit across both apps.
- How many consumers? Everything below has exactly two — until a third appears, a shared abstraction must pay for itself at two.
- Same meaning, or same shape? Two blocks that look alike but mean different things per product are a reuse trap if forced into one type.
Share only what makes the shell deeper for both apps — a simple call hiding real work. A shared wrapper that just forwards arguments is negative value no matter how much duplication it removes.
Verdicts¶
Deep-share-now¶
One-line rationale each; block letters refer to the duplication audit.
| Block | Verdict | Rationale |
|---|---|---|
| Toolbar wiring (C) | Move to shell | Ten identical lambdas in both apps (src/SlicerApp/MainWindow.axaml.cs:80-89 ↔ src/SimulatorApp/MainWindow.axaml.cs:91-100); pure UI glue with zero product divergence. |
| Camera drag math (M) | Move to shell | Bit-identical orbit/pan math (src/SlicerApp/MainWindow.axaml.cs:560-593 ↔ src/SimulatorApp/MainWindow.axaml.cs:661-692); belongs next to ViewportController as one pointer-delta entry point. |
| Overlay legend apply (L) | Move to shell | Both call _viewport.ApplyOverlay(...) then write legend min/max/mean the same way (src/SlicerApp/MainWindow.axaml.cs:516-558 ↔ src/SimulatorApp/MainWindow.axaml.cs:465-508); only the status string differs — an optional callback, not a fork. |
| Layer slider control (J) | Move to shell | Identical _maxLayer → SetLayerVisibility → BuildBuffers → label flow (src/SlicerApp/MainWindow.axaml.cs:483-502 ↔ src/SimulatorApp/MainWindow.axaml.cs:433-452); a small control exposing MaxLayer + ValueChanged fits both. |
Sim paint → GcodeToolpathPainter |
Complete the existing share | The painter was written for both apps — its maxMoveIndexExclusive parameter exists for the Simulator playhead — but only Slicer calls it (src/SlicerApp/MainWindow.axaml.cs:622-631); Sim still duplicates the ~70-line loop with its own projection/heat buffers (src/SimulatorApp/MainWindow.axaml.cs:748-817). |
Sim findings → FindingFormatter |
Complete the existing share | Slicer already projects through FindingFormatter.ProjectFromSession (src/SlicerApp/MainWindow.axaml.cs:400-405); Sim still holds raw NpsAbi summaries and calls GetFindingDetail from the window (src/SimulatorApp/MainWindow.axaml.cs:332-355). |
Keep split, with reasons¶
| Block | Why split is correct |
|---|---|
| Load / generate chain (G) | Shared spine (fixture → LoadModel → generate → analyze → overlay), but the ends are already forked: Slicer harvests SliceParams and loads warp grids, Sim hard-codes 5 demo layers, reloads the mesh, and seeds playback markers (src/SlicerApp/MainWindow.axaml.cs:192-246,321-363 vs src/SimulatorApp/MainWindow.axaml.cs:205-314). A shared LoadAnalyzePipeline would freeze both products on the demo toolpath. |
| Playback timer policy (Q) | Timer ownership, auto-pause, and scrub resume are Simulator product behavior (src/SimulatorApp/MainWindow.axaml.cs:510-658); PlaybackPanel in the shell is correctly a pure UI surface. No DispatcherTimer in the shell until a third consumer exists. |
| Slice params UI (R) | Slicer-only today and grows with the real slicer; nothing in Simulator consumes it. |
| Disposal (E) | Same shape, different owned resources — Sim stops a timer and nulls _mesh, Slicer does not (src/SlicerApp/MainWindow.axaml.cs:120-134 ↔ src/SimulatorApp/MainWindow.axaml.cs:160-185). A shared "dispose order" comment is enough. |
| MainWindow base class | Rejected. Avalonia windows cannot multiply-inherit, and the diverging product surfaces would make a base class a forced abstraction; ShellKeybindings.cs:24-27 already documents this reasoning. Prefer small composable helpers. |
Not engine¶
Orbit/pan math, Avalonia line paint, legend strings, toolbar events, key
chords, DispatcherTimer, and slice-parameter NumericUpDowns are all
shell/app policy. The engine already owns moves, overlay values, analysis,
and projection behind the ABI; pushing UI policy down would pull app
concerns across the one host border (README.md:161-168).
The stated direction: one binary, two modes¶
HOW IT SHOULD BE (STATED). ADR 0002 is Accepted: collapse the two
app projects into one Avalonia binary with a ProductMode { Slicer,
Simulator } launch argument and an IProductMode per-mode panel
registration seam, in a provisional src/Nps.App/ project
(docs/adr/0002-one-binary-product-mode.md:71-108). The Docker
entrypoint would take the mode from an env var (NPS_MODE=slicer |
simulator) or a CLI flag, and start-gui would keep two named entry
points that invoke the same binary with different arguments
(docs/adr/0002-one-binary-product-mode.md:102-107).
What changes vs today:
| Today (HOW IT IS) | After ADR 0002 (STATED) |
|---|---|
Two app projects, src/SlicerApp/ + src/SimulatorApp/ (docs/architecture.md:15-31) |
One executable project, src/Nps.App/ |
| Mode chosen by which binary you launch | Mode chosen by --mode= / env var at startup |
| Per-app XAML composes regions by hand | IProductMode registers each mode's panels into the shared shell |
Duplicate scaffolding in both MainWindows |
Deleted; one MainWindow, per-mode seams |
Accepted but orphaned
The ADR says follow-up work is "tracked as child implementation beads"
(docs/adr/0002-one-binary-product-mode.md:183-186), but no open
child beads exist — bd list shows only the docs bead nps-0m7
in flight. INFERENCE: the ProductMode collapse is decided on
paper and unowned in practice; until implementation beads appear, the
two-app tree remains the real architecture and this site documents it
as such.
Divergence forecast¶
INFERENCE (grounded in the stated product directions — real slicing
behind a future slice/ engine module, docs/architecture.md:378-425;
README physics-simulation framing, README.md:15):
| Area | Slicer trajectory | Simulator trajectory | Share future |
|---|---|---|---|
| Load / generate | Real slice params, regenerate | Demo or imported toolpath + mesh context | Stay split |
| Viewport paint | Full path / layer preview | Playhead-capped path + mesh underlay | Shared painter, different arguments |
| Findings | Slice-time defects + warp grids | Playback-linked findings + auto-pause | Shared list/detail; different navigation side-effects |
| Overlays | Same heatmap modes | Same | Deep-share now |
| Camera / toolbar | Same | Same | Deep-share now |
| Playback | None | Richer simulation | Simulator-only |
Where copying is correct — the no-dogma clause¶
Duplication is not a moral failure. Copying is the correct choice when:
- The code is simple and static.
FixtureLocator.FindPrimaryBenchy3Mf()and theStatusone-liner cost less as small shared leaves than as machinery — and if one ever forked, replicating it would be fine too. - The two sides are about to diverge anyway. The load chain is the case study: both apps generate the demo toolpath today, but Slicer is headed for real slice parameters and Simulator for playback seeding. Merging them now would create a shared type that must serve two product meanings — the reuse trap — and the break-up would cost more than the copies do.
- The shared version would be a pass-through. A shell method that
only forwards to
ViewportControllerwith the same signature adds surface without depth; delete the wrapper, keep the call sites.
The warning cut the other way too: forcing one type to serve both
products is how FindingItem.Data ended up exposing raw ABI summaries to
app code (src/Nps.Shell/Controls/FindingItem.cs:98) — convenient for
Simulator, corrosive to Slicer's no-ABI-in-the-UI goal. And
StatusSink.Prefix = "[SlicerUI]" (src/Nps.Shell/StatusSink.cs:36) is
shared by both apps: the code comment calls the unification deliberate,
but the analysis verdict is that a Slicer-branded prefix in Simulator
status output is sharing that went one step too far.
Sequenced fix list¶
Ordered so each step makes the next one smaller. Items 1–2 are wrong-sharing fixes; 3–6 are glue extraction; 7 waits for beads.
- Fix the wrong shares first. Decide the
StatusSink.Prefixpolicy (one neutral prefix, or per-app prefixes) and drain the ABI leak inFindingItem.Dataso both apps speakFindingItem, notNpsAnalysisFindingSummary. - Kill the dead twin.
OnActivityTabClickin both apps only forwards toShell.Activity.HandleClick— pure cloned dead weight. - Migrate Simulator paint to
GcodeToolpathPainter.RenderTowithmaxMoveIndexExclusive = _simMoveIndex + 1; delete Sim's_projBufA/B/_heatBufand the inlined loop. - Migrate Simulator findings to
FindingFormatter.ProjectFromSession; the Sim-only remainder is feeding markers toPlaybackPanel. - Extract the four glue helpers into the shell: toolbar bind, camera drag, overlay-legend apply, layer-slider control. Each is one deep helper; none is a base class.
- Re-measure. After 3–5 the residual per-app code should be genuine product policy (slice params, playback, load forks) — update the duplication audit numbers.
- ProductMode, when beads appear. Only then: create
src/Nps.App/, move bothMainWindows behindIProductMode, delete the scaffolding — per ADR 0002 (docs/adr/0002-one-binary-product-mode.md:69-108). Doing it before steps 1–5 would bake today's clones into the new binary permanently.
See also: what is shared today, the Simulator direction, and the Slicer direction.