refactor(api): clarify EncodedOutput metadata and EncoderConfig ownership

[rmems]

Goal

Resolve placeholder and weakly connected public types before 1.0 so downstream users do not build against semantics that are later removed or radically changed.

EncodingMetadata is currently empty, while the general EncoderConfig defaults to 256 input/output channels but is not the authoritative configuration type for most encoders.

Scope

• Audit all uses and exports of EncodingMetadata, EncodedOutput::embeddings, and EncoderConfig.
• Decide whether each type should be completed, narrowed, feature-gated, made private, deprecated, or removed before 1.0.
• Define which metadata is truly common across encoding algorithms.
• Avoid a catch-all metadata structure that accumulates framework-specific fields.
• Document configuration ownership: common configuration versus encoder-specific configuration.
• Add migration notes for any removals or replacements.

Design constraints

• Public types must have concrete, stable semantics.
• Domain-specific telemetry metadata belongs in downstream adapters, not core encoding.
• Do not force every encoder into one oversized configuration object.
• Preserve straightforward defaults where they represent real behavior.

Acceptance criteria

• Every exported field/type has a documented purpose and consumer.
• Empty placeholder public types are removed or given concrete semantics.
• EncoderConfig ownership and use are consistent across docs and examples.
• Metadata remains framework- and domain-agnostic.
• Deprecations include a migration path.
• Public API tests lock the chosen surface.

Relationships

• Depends on feat(api): define explicit spike timestamp and timebase semantics #62 because timebase information may affect common metadata design.
• Coordinate with feat(api): add allocation-reusing encode_into and SpikeSink interface #63 because sink-based encoding may change EncodedOutput's role.
• Coordinate with refactor(api): unify EmbeddingRateEncoder with the primary Encoder API #64 because embedding output/state must use the same public model.
• Blocks the eventual 1.0.0 API freeze.

---

🤖 Authored with ChatGPT: GPT-5.6 (Medium)

[linear-code]

LIM-976

[rmems]

@coderabbitai plan

[coderabbitai]

Coding Plan

Summary

• Remove the empty EncodingMetadata type, the EncodedOutput::metadata field, and the decorative EncoderConfig, because they have no concrete semantics or real consumer before 1.0.
• Keep EncodedOutput::embeddings, document its purpose, and populate it from EmbeddingRateEncoder::forward so it has a concrete consumer, coordinating with #64.
• Update all examples, serde tests, and the prelude smoke test to match the new surface, and add a dedicated public API test that locks it.
• Document per-encoder configuration ownership and the framework-agnostic metadata policy, record migration notes, and add REVIEW.md regression guards using the existing convention.

Design Choices

Design Choice 1: Resolution of the empty `EncodingMetadata` type and the `EncodedOutput::metadata` field

Options Considered:

1. Give EncodingMetadata concrete fields now (for example source_sample_index, timebase data).
2. Remove EncodingMetadata and the metadata field, and add migration notes.
3. Feature-gate the type and field behind an optional feature.

Chosen Option: 2

Rationale: No common metadata semantics exist yet, timebase design depends on #62, and shipping an empty public type before 1.0 conflicts with the goal of concrete, stable semantics.

Design Choice 2: Resolution of `EncoderConfig`

Options Considered:

1. Keep EncoderConfig and document it as an optional common channel descriptor.
2. Mark EncoderConfig #[deprecated] with a migration note, then remove after a cycle.
3. Remove EncoderConfig and document per-encoder configuration ownership, with migration notes.

Chosen Option: 3

Rationale: The type is decorative and not authoritative, the crate is pre-1.0, and removal with clear ownership documentation gives the cleanest stable surface.

Design Choice 3: Resolution of `EncodedOutput::embeddings`

Options Considered:

1. Remove the embeddings field.
2. Keep the field and document it as a reserved slot with the consumer deferred to #64.
3. Keep the field, document it, and wire a minimal real consumer (EmbeddingRateEncoder) that populates it.

Chosen Option: 3

Rationale: This gives the field a documented purpose and a concrete consumer now, satisfies the acceptance criteria, and aligns with #64 without performing the full trait unification that #64 owns.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Resolve the public types in src/types.rs and src/encoder.rs

This phase makes the core API decisions concrete. It removes the empty and decorative types, documents the fields that remain, and gives embeddings a real consumer. Group these changes together because they define the public surface that later phases lock and document.

Task 1: Remove `EncodingMetadata` and the `EncodedOutput::metadata` field

Remove the empty placeholder type so no downstream code builds against undefined semantics.

• Delete the EncodingMetadata struct definition from src/types.rs.
• Remove the metadata: Option<EncodingMetadata> field from EncodedOutput in src/types.rs.
• Update the EncodedOutput unit tests in src/types.rs that assert on the metadata field.
• Confirm EncodedOutput::new and its Default derive still compile after the field removal.

Task 2: Document and populate `EncodedOutput::embeddings`

Keep embeddings as the standardized slot for embedding-producing encoders and give it a concrete consumer.

• Add a doc comment to the embeddings field in src/types.rs. State its purpose (optional dense embedding vector produced alongside spikes) and its consumer.
• Update EmbeddingRateEncoder::forward in src/encoder.rs so the returned EncodedOutput sets embeddings to the normalized embedding vector.
• Add a doc comment near EmbeddingRateEncoder::forward that states it populates EncodedOutput::embeddings, and note that full trait-model unification is tracked by #64.

Task 3: Remove `EncoderConfig` and record configuration ownership

Remove the decorative shared config type and make encoder-specific ownership explicit.

• Delete the EncoderConfig struct and its Default impl from src/types.rs.
• Remove the EncoderConfig default-value unit test from src/types.rs.
• Add module-level documentation in src/types.rs that states each encoder owns its own configuration through its constructor parameters, and that there is no shared configuration object.

🤖 Prompt for AI agents

Implement Phase 1: resolve the core public API decisions in `src/types.rs` and
`src/encoder.rs`. The goal is to remove placeholder or decorative types and give
the remaining fields documented, concrete purposes.

- In `src/types.rs`, delete the `EncodingMetadata` struct definition entirely.
- In `src/types.rs`, remove the `metadata: Option<EncodingMetadata>` field from
`EncodedOutput`.
- Update the `EncodedOutput` unit tests in `src/types.rs` to remove assertions
on the `metadata` field.
- Verify `EncodedOutput::new` and its `Default` derive still compile after
removing the `metadata` field.
- Add a doc comment to the `embeddings` field on `EncodedOutput` in
`src/types.rs` stating its purpose (optional dense embedding vector produced
alongside spikes) and its consumer.
- Update `EmbeddingRateEncoder::forward` in `src/encoder.rs` so the returned
`EncodedOutput` sets the `embeddings` field to the normalized embedding vector
(`normalized_embeddings`).
- Add a doc comment near `EmbeddingRateEncoder::forward` stating that it
populates `EncodedOutput::embeddings`, and note that full trait-model
unification is tracked by issue `#64`.
- Delete the `EncoderConfig` struct and its `Default` impl from `src/types.rs`.
- Remove the `EncoderConfig` default-value unit test from `src/types.rs`.
- Add module-level documentation in `src/types.rs` stating that each encoder
owns its own configuration through its constructor parameters, and that there is
no shared configuration object.

Phase 2: Update dependents so the crate compiles and reflects the new surface

This phase updates every example and test that referenced the removed or changed types. Group these changes together because they all follow from the Phase 1 decisions and must land in the same change to keep the build green.

Task 1: Update the prelude smoke test and examples

Remove references to the deleted EncoderConfig type.

• Update test_lib_prelude_imports in src/lib.rs to remove the EncoderConfig::default() reference. Keep a prelude-import smoke check for a type that still exists.
• Update examples/rate_encoding.rs to stop using EncoderConfig. Drive the demo input vector size from a local constant and describe the architecture without the removed type.

Task 2: Update serde tests

Remove serde coverage for the deleted types and keep coverage for embeddings.

• Update tests/serde_tests.rs to remove the EncoderConfig round-trip block and the EncodingMetadata usage.
• Keep an EncodedOutput round-trip that sets spikes and embeddings, and remove the metadata assignment.

Task 3: Add a public API test that locks the chosen surface

Guard the resolved surface against unintended change.

• Create tests/public_api.rs. Assert that EncodedOutput exposes spikes and embeddings and can be constructed via EncodedOutput::new.
• Assert that EncoderConfig and EncodingMetadata are no longer importable from the prelude, using a compile-checked reference to the types that remain.
• Assert that EmbeddingRateEncoder::forward returns an EncodedOutput whose embeddings field is populated.

🤖 Prompt for AI agents

Implement Phase 2: update every dependent file so the crate compiles cleanly
after the Phase 1 type removals and changes, and add a test that locks the new
public surface.

- In `src/lib.rs`, update `test_lib_prelude_imports` to remove the
`EncoderConfig::default()` reference, and keep a prelude-import smoke check
using a type that still exists.
- In `examples/rate_encoding.rs`, remove the use of `EncoderConfig`. Drive the
demo input vector size from a local constant and update the architecture
description to not reference the removed type.
- In `tests/serde_tests.rs`, remove the `EncoderConfig` round-trip test block
and the `EncodingMetadata` usage. Keep an `EncodedOutput` round-trip test that
sets `spikes` and `embeddings`, and remove the `metadata` assignment.
- Create `tests/public_api.rs` as a new test file that:
- Asserts `EncodedOutput` exposes `spikes` and `embeddings` fields and can be
constructed via `EncodedOutput::new`.
- Asserts `EncoderConfig` and `EncodingMetadata` are no longer importable from
the prelude, using a compile-checked reference to the types that remain.
- Asserts `EmbeddingRateEncoder::forward` returns an `EncodedOutput` whose
`embeddings` field is populated.

Phase 3: Documentation, migration notes, and regression guards

This phase documents the ownership model, records the migration path for the removed types, and extends the existing regression-guard convention. Group these changes together because they finalize the audit trail required by the acceptance criteria.

Task 1: Document configuration ownership and metadata policy

Make ownership and metadata scope explicit in project docs.

• Update README.md and docs/architecture-siblings.md to state that each encoder owns its own configuration and that there is no shared EncoderConfig.
• Document that EncodedOutput carries spikes and an optional embeddings vector only. State that domain-specific or biological telemetry belongs in downstream adapters, consistent with the crate-boundary ownership rules.

Task 2: Add migration notes for the removals

Record the migration path for EncodingMetadata and EncoderConfig.

• Create a CHANGELOG.md (or a migration section in README.md). List the removal of EncodingMetadata, the EncodedOutput::metadata field, and EncoderConfig.
• For each removal, state the replacement or rationale. For EncoderConfig, direct users to each encoder's own constructor. For metadata, note that common metadata design is deferred to #62.

Task 3: Update `REVIEW.md` regression guards

Extend the existing guard convention to lock the new surface.

• Add a regression-guard entry to REVIEW.md that greps src/types.rs to confirm EncoderConfig and EncodingMetadata are absent.
• Add a guard that confirms EncodedOutput still exposes spikes and embeddings, matching the existing rg-based guard style.

🤖 Prompt for AI agents

Implement Phase 3: document the ownership model, record migration notes for the
removed types, and extend the regression-guard convention.

- Update `README.md` and `docs/architecture-siblings.md` to state that each
encoder owns its own configuration and that there is no shared `EncoderConfig`.
- Document that `EncodedOutput` carries `spikes` and an optional `embeddings`
vector only, and that domain-specific or biological telemetry belongs in
downstream adapters, consistent with existing crate-boundary ownership rules.
- Create a `CHANGELOG.md` file, or add a migration section to `README.md`,
listing the removal of `EncodingMetadata`, the `EncodedOutput::metadata` field,
and `EncoderConfig`.
- For each removed item, state the replacement or rationale: for
`EncoderConfig`, direct users to each encoder's own constructor; for `metadata`,
note that common metadata design is deferred to issue `#62`.
- In `REVIEW.md`, add a regression-guard entry that greps `src/types.rs` to
confirm `EncoderConfig` and `EncodingMetadata` are absent.
- In `REVIEW.md`, add a guard confirming `EncodedOutput` still exposes `spikes`
and `embeddings`, matching the existing `rg`-based guard style used in the file.

Research

axon-encoder (version 0.4.0, edition 2024) exposes encoders through the Encoder and ModulatedEncoder traits defined in src/lib.rs. The standardized output type EncodedOutput and the shared EncoderConfig/EncodingMetadata types live in src/types.rs and are re-exported only through pub mod prelude. The eight concrete encoders in src/encoders/*.rs each own their configuration as struct fields and constructor parameters. None of them use EncoderConfig, and none populate EncodedOutput::embeddings or EncodedOutput::metadata; those fields stay None. EncodingMetadata is an empty placeholder struct. EncoderConfig (default 256/256 channels) is decorative and is only referenced in examples/rate_encoding.rs, a prelude smoke test, and serde round-trip tests. src/encoder.rs holds a separate EmbeddingRateEncoder that returns (EncodedOutput, EncoderState) and normalizes an embedding vector into normalized_embeddings, but never writes it to EncodedOutput::embeddings. docs/architecture-siblings.md states that domain-specific and biological telemetry belongs to downstream adapters, not core encoding. REVIEW.md protects public API with grep and line-count regression guards. There is no #[deprecated] precedent and no CHANGELOG.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement Phase 1: resolve the core public API decisions in `src/types.rs` and
`src/encoder.rs`. The goal is to remove placeholder or decorative types and give
the remaining fields documented, concrete purposes.

- In `src/types.rs`, delete the `EncodingMetadata` struct definition entirely.
- In `src/types.rs`, remove the `metadata: Option<EncodingMetadata>` field from
`EncodedOutput`.
- Update the `EncodedOutput` unit tests in `src/types.rs` to remove assertions
on the `metadata` field.
- Verify `EncodedOutput::new` and its `Default` derive still compile after
removing the `metadata` field.
- Add a doc comment to the `embeddings` field on `EncodedOutput` in
`src/types.rs` stating its purpose (optional dense embedding vector produced
alongside spikes) and its consumer.
- Update `EmbeddingRateEncoder::forward` in `src/encoder.rs` so the returned
`EncodedOutput` sets the `embeddings` field to the normalized embedding vector
(`normalized_embeddings`).
- Add a doc comment near `EmbeddingRateEncoder::forward` stating that it
populates `EncodedOutput::embeddings`, and note that full trait-model
unification is tracked by issue `#64`.
- Delete the `EncoderConfig` struct and its `Default` impl from `src/types.rs`.
- Remove the `EncoderConfig` default-value unit test from `src/types.rs`.
- Add module-level documentation in `src/types.rs` stating that each encoder
owns its own configuration through its constructor parameters, and that there is
no shared configuration object.
===============================================================================

Task: 2

Implement Phase 2: update every dependent file so the crate compiles cleanly
after the Phase 1 type removals and changes, and add a test that locks the new
public surface.

- In `src/lib.rs`, update `test_lib_prelude_imports` to remove the
`EncoderConfig::default()` reference, and keep a prelude-import smoke check
using a type that still exists.
- In `examples/rate_encoding.rs`, remove the use of `EncoderConfig`. Drive the
demo input vector size from a local constant and update the architecture
description to not reference the removed type.
- In `tests/serde_tests.rs`, remove the `EncoderConfig` round-trip test block
and the `EncodingMetadata` usage. Keep an `EncodedOutput` round-trip test that
sets `spikes` and `embeddings`, and remove the `metadata` assignment.
- Create `tests/public_api.rs` as a new test file that:
- Asserts `EncodedOutput` exposes `spikes` and `embeddings` fields and can be
constructed via `EncodedOutput::new`.
- Asserts `EncoderConfig` and `EncodingMetadata` are no longer importable from
the prelude, using a compile-checked reference to the types that remain.
- Asserts `EmbeddingRateEncoder::forward` returns an `EncodedOutput` whose
`embeddings` field is populated.
===============================================================================

Task: 3

Implement Phase 3: document the ownership model, record migration notes for the
removed types, and extend the regression-guard convention.

- Update `README.md` and `docs/architecture-siblings.md` to state that each
encoder owns its own configuration and that there is no shared `EncoderConfig`.
- Document that `EncodedOutput` carries `spikes` and an optional `embeddings`
vector only, and that domain-specific or biological telemetry belongs in
downstream adapters, consistent with existing crate-boundary ownership rules.
- Create a `CHANGELOG.md` file, or add a migration section to `README.md`,
listing the removal of `EncodingMetadata`, the `EncodedOutput::metadata` field,
and `EncoderConfig`.
- For each removed item, state the replacement or rationale: for
`EncoderConfig`, direct users to each encoder's own constructor; for `metadata`,
note that common metadata design is deferred to issue `#62`.
- In `REVIEW.md`, add a regression-guard entry that greps `src/types.rs` to
confirm `EncoderConfig` and `EncodingMetadata` are absent.
- In `REVIEW.md`, add a guard confirming `EncodedOutput` still exposes `spikes`
and `embeddings`, matching the existing `rg`-based guard style used in the file.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!