Rename Recorder -> Probe (the edge tap); decide whether a Probe must heap-allocate a record per cycle #77

Closed
opened 2026-06-17 09:52:54 +02:00 by Brummel · 4 comments
Owner

Found in the code-level performance audit and reframed after a naming/ontology pass: today's Recorder is misnamed — it is not a result-accumulator, it is the probe (the "multimeter" you clip onto an edge to read the value in flight, for the playground's live view and for test assertions). It is a pure consumer (C8 sink) that taps an edge without altering the flow.

This issue is two things.

1. Rename Recorder -> Probe (and the colliding fixture)

  • crates/aura-std/src/recorder.rs -> probe.rs; struct Recorder -> Probe; Recorder::{new,builder} -> Probe::{new,builder}; the lib.rs re-export and every call site across the workspace (~60 construction sites + the test taps).
  • Fixture collision: struct Probe(Vec<Cell>) already exists as a private test fixture in crates/aura-core/src/node.rs:412 (a node whose label() echoes the param vector it received — named in spec 0047). It must be renamed in the same change (e.g. ParamEcho) so there are not two Probe node types.
  • Naming rationale (a 4-lens panel + synthesis): Probe models a handle attached to an edge honestly (hardware probe, LabVIEW Probe, Akka TestSink.probe) and matches the multimeter image. Tap was the runner-up but rejected: in RxJS/Kafka/tee/PassThrough "tap" denotes an inline pass-through (one in, one out), whereas this node is a terminal sink with no output port — Tap would imply the value flows through it, and the TapForward fixture cements exactly that wrong intuition.
  • Glossary: add a Probe entry (observation tap), keep sink as the umbrella term; add an Avoid line steering Recorder/Tap/Monitor/Scope away from this slot.

2. Must a Probe heap-allocate a record per cycle to forward it?

This is the original performance finding, now correctly scoped to the probe. Today eval does let row = Vec::with_capacity(kinds.len()) + tx.send((now, row)) every fired cycle — a heap allocation per recorded row, on every run that taps an edge, sending 16-byte Scalars.

It is not unavoidable. The per-cycle Vec is an artifact of two independent choices, each avoidable:

  • (A) Drop the channel, accumulate internally. If the probe does not need to stream live (true today — tests and metrics read everything at the end), it owns growing Vec<Cell> columns and pushes per cycle (amortized O(1), zero malloc per cycle — only occasional realloc as it grows). The consumer reads the columns after the run; needs a non-channel way out of the type-erased graph (Arc<Mutex<…>> handle, or a harness node-accessor).
  • (B) Keep the channel for live view, but stop building a Vec each cycle. Send Cell (8B, Copy) not Scalar (16B); for the dominant single-column probe the unit is a bare Cell — no heap at all. Only a wide multi-column probe would still pay.

The genuinely unavoidable heap is the narrow case only: live-streaming a variable-width multi-column row. The single-column case and the accumulate-then-read case are both per-cycle allocation-free.

Decision (channel-vs-accumulate, Cell-vs-Scalar, live-vs-batch) belongs in a brainstorm/specify cycle, not a freehand edit. Blast radius note: the channel is also the engine's universal test-tap (~80 rx.try_iter().collect() sites in harness.rs/blueprint.rs), so whichever delivery shape wins, the migration touches the whole test surface — scope it as its own cycle.

Sibling audit findings: O(1) SMA (#39, done) and ingest double-materialization (#78, done).

Found in the code-level performance audit and reframed after a naming/ontology pass: today's `Recorder` is misnamed — it is not a result-accumulator, it is the **probe** (the "multimeter" you clip onto an edge to read the value in flight, for the playground's live view and for test assertions). It is a pure consumer (C8 sink) that taps an edge without altering the flow. This issue is two things. ## 1. Rename `Recorder` -> `Probe` (and the colliding fixture) - `crates/aura-std/src/recorder.rs` -> `probe.rs`; `struct Recorder` -> `Probe`; `Recorder::{new,builder}` -> `Probe::{new,builder}`; the `lib.rs` re-export and every call site across the workspace (~60 construction sites + the test taps). - **Fixture collision:** `struct Probe(Vec<Cell>)` already exists as a private test fixture in `crates/aura-core/src/node.rs:412` (a node whose `label()` echoes the param vector it received — named in spec 0047). It must be renamed in the same change (e.g. `ParamEcho`) so there are not two `Probe` node types. - Naming rationale (a 4-lens panel + synthesis): `Probe` models a handle *attached to* an edge honestly (hardware probe, LabVIEW Probe, Akka `TestSink.probe`) and matches the multimeter image. `Tap` was the runner-up but rejected: in RxJS/Kafka/tee/`PassThrough` "tap" denotes an **inline pass-through** (one in, one out), whereas this node is a terminal sink with no output port — `Tap` would imply the value flows *through* it, and the `TapForward` fixture cements exactly that wrong intuition. - Glossary: add a `Probe` entry (observation tap), keep `sink` as the umbrella term; add an Avoid line steering `Recorder`/`Tap`/`Monitor`/`Scope` away from this slot. ## 2. Must a `Probe` heap-allocate a record per cycle to forward it? This is the original performance finding, now correctly scoped to the probe. Today `eval` does `let row = Vec::with_capacity(kinds.len())` + `tx.send((now, row))` every fired cycle — a heap allocation per recorded row, on every run that taps an edge, sending 16-byte `Scalar`s. It is **not** unavoidable. The per-cycle `Vec` is an artifact of two independent choices, each avoidable: - **(A) Drop the channel, accumulate internally.** If the probe does not need to stream live (true today — tests and metrics read everything at the end), it owns growing `Vec<Cell>` columns and pushes per cycle (amortized O(1), **zero malloc per cycle** — only occasional realloc as it grows). The consumer reads the columns after the run; needs a non-channel way out of the type-erased graph (`Arc<Mutex<…>>` handle, or a harness node-accessor). - **(B) Keep the channel for live view, but stop building a `Vec` each cycle.** Send `Cell` (8B, `Copy`) not `Scalar` (16B); for the dominant **single-column** probe the unit is a bare `Cell` — no heap at all. Only a wide multi-column probe would still pay. The genuinely unavoidable heap is the narrow case only: **live-streaming a variable-width multi-column row**. The single-column case and the accumulate-then-read case are both per-cycle allocation-free. Decision (channel-vs-accumulate, `Cell`-vs-`Scalar`, live-vs-batch) belongs in a brainstorm/specify cycle, not a freehand edit. Blast radius note: the channel is also the engine's universal **test-tap** (~80 `rx.try_iter().collect()` sites in `harness.rs`/`blueprint.rs`), so whichever delivery shape wins, the migration touches the whole test surface — scope it as its own cycle. Sibling audit findings: O(1) SMA (#39, done) and ingest double-materialization (#78, done).
Brummel added the idea label 2026-06-17 09:52:54 +02:00
Brummel changed title from Recorder::eval heap-allocates a Vec + mpsc send per cycle (only per-cycle alloc in a std node) to Rename Recorder -> Probe (the edge tap); decide whether a Probe must heap-allocate a record per cycle 2026-06-17 10:49:26 +02:00
Author
Owner

Cross-reference: #138 / cycle 0070 built the lifecycle primitive this issue's option (A) needs

The BLOCKER #138 fix (M3 streaming sink reductions, cycle 0070) added a
Node::finalize(&mut self) end-of-stream lifecycle — the engine calls it once per
node in topological order after the source loop drains — which is exactly the
"non-channel way out of the type-erased graph" this issue's option (A)
(accumulate internally, read after the run) named as missing. It is realized over
the existing channel (a single compact flush at stream end), a third delivery
shape not enumerated here.

What #138 did NOT touch: this issue's part 1 (rename Recorder -> Probe) and
the Recorder's own per-cycle allocation — it left Recorder as-is (per-cycle
Vec + Scalar send) for the live / --trace / test-tap path, and shipped new
folding sinks (GatedRecorder, SeriesReducer) for the reduction path instead.

So this issue stays open, but de-risked: the lifecycle is proven and shipped, and
the rename can fold the new sinks into a clean Probe ontology. Ledger C8 now records
the finalize lifecycle.

## Cross-reference: #138 / cycle 0070 built the lifecycle primitive this issue's option (A) needs The BLOCKER #138 fix (M3 streaming sink reductions, cycle 0070) added a `Node::finalize(&mut self)` end-of-stream lifecycle — the engine calls it once per node in topological order after the source loop drains — which is exactly the "non-channel way out of the type-erased graph" this issue's **option (A)** (accumulate internally, read after the run) named as missing. It is realized over the *existing* channel (a single compact flush at stream end), a third delivery shape not enumerated here. What #138 did NOT touch: this issue's **part 1** (rename `Recorder` -> `Probe`) and the `Recorder`'s own per-cycle allocation — it left `Recorder` as-is (per-cycle `Vec` + `Scalar` send) for the live / `--trace` / test-tap path, and shipped *new* folding sinks (`GatedRecorder`, `SeriesReducer`) for the reduction path instead. So this issue stays open, but de-risked: the lifecycle is proven and shipped, and the rename can fold the new sinks into a clean Probe ontology. Ledger C8 now records the `finalize` lifecycle.
Author
Owner

Triage 2026-07-09 (tree at 68317ec) — premise intact, but the naming half predates two vocabulary landings the eventual cycle must reconcile:

  • Unchanged: struct Recorder with the per-cycle Vec::with_capacity row (crates/aura-std/src/recorder.rs:24/:68); the colliding test fixture struct Probe (crates/aura-core/src/node.rs:498); Node::finalize exists (node.rs:350), keeping the accumulate-then-flush option open. Recorder is deliberately absent from the blueprint vocabulary (crates/aura-std/src/vocabulary.rs:110 pins std_vocabulary("Recorder") to None), so the rename has no serialized-data surface — a pure Rust-symbol migration (harness.rs alone has ~51 Recorder hits).
  • New since filing: "tap" is now a load-bearing glossary term (a named recorded stream; the persist_taps closed vocabulary), and the CLI uses "probe" in an unrelated sweep-terminal sense (blueprint_axis_probe). The 2026-06-17 rationale for the name "Probe" needs re-validation against both before any rename executes.
Triage 2026-07-09 (tree at 68317ec) — premise intact, but the naming half predates two vocabulary landings the eventual cycle must reconcile: - Unchanged: struct Recorder with the per-cycle `Vec::with_capacity` row (crates/aura-std/src/recorder.rs:24/:68); the colliding test fixture `struct Probe` (crates/aura-core/src/node.rs:498); Node::finalize exists (node.rs:350), keeping the accumulate-then-flush option open. Recorder is deliberately absent from the blueprint vocabulary (crates/aura-std/src/vocabulary.rs:110 pins std_vocabulary("Recorder") to None), so the rename has no serialized-data surface — a pure Rust-symbol migration (harness.rs alone has ~51 Recorder hits). - New since filing: "tap" is now a load-bearing glossary term (a named recorded stream; the persist_taps closed vocabulary), and the CLI uses "probe" in an unrelated sweep-terminal sense (blueprint_axis_probe). The 2026-06-17 rationale for the name "Probe" needs re-validation against both before any rename executes.
Collaborator

Re-anchoring note after the #295 shell-boundary extraction (crate carve merged as commit 5c2ac98, 2026-07-21) and the document-first direction ratified in #300 — the substance of this issue is unchanged and still wanted; only file locations have moved.

Fixture collision: the colliding test fixture struct Probe(Vec<Cell>) is no longer at crates/aura-core/src/node.rs:412; it is currently at crates/aura-core/src/node.rs:534 (unrelated growth in the surrounding module, not a crate move).

Call-site distribution: before #295, the bulk of Recorder construction sites lived inside the CLI shell (crates/aura-cli/src/main.rs and campaign_run.rs). #295 extracted that logic into library crates, so the rename's actual blast radius today is:

  • crates/aura-std/src/recorder.rs (the definition itself) and crates/aura-std/src/gated_recorder.rs (GatedRecorder, a sibling sink sharing the Recorder stem — not mentioned in the original issue text, but a consistent rename should cover it too, e.g. GatedProbe)
  • crates/aura-runner/src/measure.rs and crates/aura-runner/src/member.rs (run_signal_r, run_blueprint_member, and the probe/reopen cluster)
  • crates/aura-backtest/src/mc.rs, crates/aura-backtest/src/metrics.rs, crates/aura-backtest/src/position_management.rs
  • crates/aura-cli/tests/graph_construct.rs and crates/aura-cli/tests/fixtures/sample-model.json (the only remaining shell-side references)

So the rename is now cross-crate library work rather than concentrated in a shrinking CLI shell — this reinforces the issue's own note that it should be scoped as its own cycle, not a freehand edit.

Part 2 (the per-cycle heap allocation) is unchanged and still fully live: Recorder::eval in crates/aura-std/src/recorder.rs still does Vec::with_capacity(self.kinds.len()) + push + tx.send(...) every fired cycle, exactly as described. docs/design/INDEX.md:331-335 already documents this decision as open in near-identical language, confirming it is tracked correctly.

Cross-reference: #283 (drain taps during the run) bears directly on this issue's option (B) — it proposes replacing buffer-then-drain with a live consumer for the same channel this issue's Recorder feeds. #283 also cites a now-stale location for run_signal_r (crates/aura-cli/src/main.rs; it is now crates/aura-runner/src/member.rs post-#295). Whoever picks up #77's allocation question should read #283 alongside it — the two decisions (rename+alloc shape here, delivery/draining shape there) are adjacent enough to resolve in the same brainstorm/specify pass.

Re-anchoring note after the #295 shell-boundary extraction (crate carve merged as commit 5c2ac98, 2026-07-21) and the document-first direction ratified in #300 — the substance of this issue is unchanged and still wanted; only file locations have moved. Fixture collision: the colliding test fixture `struct Probe(Vec<Cell>)` is no longer at crates/aura-core/src/node.rs:412; it is currently at crates/aura-core/src/node.rs:534 (unrelated growth in the surrounding module, not a crate move). Call-site distribution: before #295, the bulk of `Recorder` construction sites lived inside the CLI shell (crates/aura-cli/src/main.rs and campaign_run.rs). #295 extracted that logic into library crates, so the rename's actual blast radius today is: - crates/aura-std/src/recorder.rs (the definition itself) and crates/aura-std/src/gated_recorder.rs (`GatedRecorder`, a sibling sink sharing the `Recorder` stem — not mentioned in the original issue text, but a consistent rename should cover it too, e.g. `GatedProbe`) - crates/aura-runner/src/measure.rs and crates/aura-runner/src/member.rs (run_signal_r, run_blueprint_member, and the probe/reopen cluster) - crates/aura-backtest/src/mc.rs, crates/aura-backtest/src/metrics.rs, crates/aura-backtest/src/position_management.rs - crates/aura-cli/tests/graph_construct.rs and crates/aura-cli/tests/fixtures/sample-model.json (the only remaining shell-side references) So the rename is now cross-crate library work rather than concentrated in a shrinking CLI shell — this reinforces the issue's own note that it should be scoped as its own cycle, not a freehand edit. Part 2 (the per-cycle heap allocation) is unchanged and still fully live: `Recorder::eval` in crates/aura-std/src/recorder.rs still does `Vec::with_capacity(self.kinds.len())` + `push` + `tx.send(...)` every fired cycle, exactly as described. docs/design/INDEX.md:331-335 already documents this decision as open in near-identical language, confirming it is tracked correctly. Cross-reference: #283 (drain taps during the run) bears directly on this issue's option (B) — it proposes replacing buffer-then-drain with a live consumer for the same channel this issue's Recorder feeds. #283 also cites a now-stale location for `run_signal_r` (`crates/aura-cli/src/main.rs`; it is now crates/aura-runner/src/member.rs post-#295). Whoever picks up #77's allocation question should read #283 alongside it — the two decisions (rename+alloc shape here, delivery/draining shape there) are adjacent enough to resolve in the same brainstorm/specify pass.
Collaborator

Design reconciliation (specify)

Spec: tap-subscribers (in production, combined cycle with #283). This issue's part 1 (rename Recorder -> Probe) is still listed open; this records its resolution.

  • Fork: Recorder -> Probe rename -> RETIRED. Recorder keeps its name; no Probe type is introduced; the colliding test fixture (struct Probe, crates/aura-core/src/node.rs:534) stays as-is; GatedRecorder keeps its stem.
    Basis: derived - the 2026-06-17 naming rationale predates two vocabulary landings the 2026-07-09 triage itself flagged for re-validation, and both now argue against the rename: (1) since C27 (#282) the declared tap occupies exactly the "handle clipped onto an edge" niche the name Probe was chosen for - the observation point is the tap, not the sink bound behind it; (2) "probe" is already load-bearing in the unrelated sweep-terminal sense (blueprint_axis_probe, binding::probe_binding), so a Probe sink would create the two-meanings collision the original rationale tried to avoid; (3) the #283 subscriber ontology names the lossless consumer a recorder - under that model today's Recorder is correctly named (it records a series), and the "misnamed result-accumulator" critique dissolves. Glossary follow-up: an Avoid line steering "probe" away from the observation-tap slot, instead of the Probe entry this issue originally planned.

  • Fork: per-cycle heap allocation (part 2) -> resolved by the #283 subscription model, specced in the same cycle. Declared taps are single-column by construction (one output field -> one scalar column), so the tap path's record sink sends (Timestamp, Cell) (16 B, Copy, zero per-cycle heap) on a bounded channel, and folds accumulate in-graph (the SeriesReducer precedent) emitting one row on finalize. This decides the issue's channel-vs-accumulate / Cell-vs-Scalar / live-vs-batch question: Cell payload + consume-during-run for full traces, in-graph accumulate for aggregates.

  • Deliberately out of scope this cycle: the legacy Recorder surface (eq/ex/r wrap sinks, the --trace path, ~80 rx.try_iter().collect() test-tap sites) is untouched. Migrating that surface onto the new delivery shape is its own follow-up cycle, per this issue's own scoping note ("the migration touches the whole test surface - scope it as its own cycle"); a follow-up issue will be filed at cycle close.

## Design reconciliation (specify) Spec: tap-subscribers (in production, combined cycle with #283). This issue's part 1 (rename Recorder -> Probe) is still listed open; this records its resolution. - **Fork: Recorder -> Probe rename** -> RETIRED. Recorder keeps its name; no `Probe` type is introduced; the colliding test fixture (`struct Probe`, `crates/aura-core/src/node.rs:534`) stays as-is; `GatedRecorder` keeps its stem. Basis: derived - the 2026-06-17 naming rationale predates two vocabulary landings the 2026-07-09 triage itself flagged for re-validation, and both now argue against the rename: (1) since C27 (#282) the **declared tap** occupies exactly the "handle clipped onto an edge" niche the name Probe was chosen for - the observation point is the tap, not the sink bound behind it; (2) "probe" is already load-bearing in the unrelated sweep-terminal sense (`blueprint_axis_probe`, `binding::probe_binding`), so a Probe sink would create the two-meanings collision the original rationale tried to avoid; (3) the #283 subscriber ontology names the lossless consumer a *recorder* - under that model today's Recorder is correctly named (it records a series), and the "misnamed result-accumulator" critique dissolves. Glossary follow-up: an Avoid line steering "probe" away from the observation-tap slot, instead of the Probe entry this issue originally planned. - **Fork: per-cycle heap allocation (part 2)** -> resolved by the #283 subscription model, specced in the same cycle. Declared taps are single-column by construction (one output field -> one scalar column), so the tap path's record sink sends `(Timestamp, Cell)` (16 B, Copy, zero per-cycle heap) on a bounded channel, and folds accumulate in-graph (the SeriesReducer precedent) emitting one row on `finalize`. This decides the issue's channel-vs-accumulate / Cell-vs-Scalar / live-vs-batch question: Cell payload + consume-during-run for full traces, in-graph accumulate for aggregates. - **Deliberately out of scope this cycle:** the legacy `Recorder` surface (eq/ex/r wrap sinks, the `--trace` path, ~80 `rx.try_iter().collect()` test-tap sites) is untouched. Migrating that surface onto the new delivery shape is its own follow-up cycle, per this issue's own scoping note ("the migration touches the whole test surface - scope it as its own cycle"); a follow-up issue will be filed at cycle close.
claude self-assigned this 2026-07-21 18:02:43 +02:00
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Brummel/Aura#77