feat(audio-datasets): agent-ready initial-manifest stages (FLEURS, ReadSpeech)#2180
feat(audio-datasets): agent-ready initial-manifest stages (FLEURS, ReadSpeech)#2180shubhamNvidia wants to merge 2 commits into
Conversation
…idency resolver Shared base imported by the audio stage modules to make them agent-ready: - _agent_ready.py: AgentReady mixin, StageContract/IOSpec/Gates/Role - _residency.py: input residency resolver (file/waveform/auto) - common.py: agent-ready helpers (ensure_mono/ensure_waveform_2d) Excludes the agent orchestration layer (registry/catalog/conformance/planning/agent).
…eContract + residency)
shubhamNvidia
requested review from
praateekmahajan
and removed request for
a team
Greptile SummaryThis PR makes the FLEURS and ReadSpeech initial-manifest stages agent-ready by introducing two new modules (
Confidence Score: 4/5Safe to merge if the test suite confirms describe_static() is not called anywhere; the processing logic is entirely unchanged and the additive contracts are correct for the instance path. The manifest-creation logic for FLEURS and ReadSpeech is untouched, and all five instance-level describe() implementations look correct. The main concern is that describe_static() — a publicly advertised method on the new AgentReady mixin — will raise ImportError on every call because _agent_registry.py is absent from the repository. If agents or tooling rely on the static-discovery path, this will fail silently until exercised. _agent_ready.py deserves another look around describe_static() and the _agent_registry dependency; _residency.py's _as_soundfile_array channel heuristic is worth verifying against short test fixtures. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Agent
participant Stage as AgentReady Stage
participant Registry as _agent_registry (missing)
Note over Agent,Stage: Instance-level discovery (works)
Agent->>Stage: stage.describe()
Stage-->>Agent: StageContract(writes, gates, cardinality)
Note over Agent,Stage: Class-level discovery (broken)
Agent->>Stage: Stage.describe_static()
Stage->>Registry: import static_contract
Registry-->>Stage: ImportError
Note over Agent,Stage: Normal pipeline execution (unaffected)
Agent->>Stage: stage.process(task)
Stage-->>Agent: AudioTask(audio_filepath, text, ...)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Agent
participant Stage as AgentReady Stage
participant Registry as _agent_registry (missing)
Note over Agent,Stage: Instance-level discovery (works)
Agent->>Stage: stage.describe()
Stage-->>Agent: StageContract(writes, gates, cardinality)
Note over Agent,Stage: Class-level discovery (broken)
Agent->>Stage: Stage.describe_static()
Stage->>Registry: import static_contract
Registry-->>Stage: ImportError
Note over Agent,Stage: Normal pipeline execution (unaffected)
Agent->>Stage: stage.process(task)
Stage-->>Agent: AudioTask(audio_filepath, text, ...)
Reviews (1): Last reviewed commit: "feat(audio-datasets): make datasets stag..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @classmethod | ||
| def describe_static(cls) -> StageContract: | ||
| """Instance-free contract for discovery/planning. | ||
|
|
||
| Uses class defaults + ``AGENT_STATIC`` and never runs ``__init__`` side | ||
| effects, so it is safe for stages with required constructor args. Prefer | ||
| the instance-level :meth:`describe` (or | ||
| ``_agent_registry.build_contract``) when resolved key *values* are | ||
| needed. | ||
| """ | ||
| from nemo_curator.stages.audio._agent_registry import static_contract | ||
|
|
||
| return static_contract(cls) |
There was a problem hiding this comment.
describe_static() always raises ImportError
describe_static() lazily imports static_contract from nemo_curator.stages.audio._agent_registry, but _agent_registry.py does not exist in the repository — it is the only file referenced from _agent_ready.py that is absent. Any call to AgentReady.describe_static() (e.g., CreateInitialManifestFleursStage.describe_static()) will immediately raise ImportError: cannot import name 'static_contract' from 'nemo_curator.stages.audio._agent_registry', making the entire static-discovery path broken. The instance-level describe() is unaffected, but this makes the describe_static / AGENT_STATIC class-variable machinery dead code right now.
| if getattr(waveform, "ndim", 0) == 2: # noqa: PLR2004 - 2 == a (channels, samples) 2-D array | ||
| channels, samples = waveform.shape | ||
| if channels == 1: | ||
| return waveform[0] | ||
| if channels < samples: | ||
| return waveform.T |
There was a problem hiding this comment.
The heuristic channels < samples used to detect whether the waveform is stored in (channels, samples) vs (samples, channels) layout breaks for any clip shorter than the channel count. For example, a 2-channel, 1-sample waveform has channels=2, samples=1; 2 < 1 is False, so the transpose is skipped and the array is passed to sf.write in the wrong orientation, producing corrupted output. In practice audio clips are long, but this could silently corrupt temp WAVs generated from synthetic or test fixtures with very few samples.
| if value is None or isinstance(value, (str, int, float, bool)): | ||
| return value | ||
| if isinstance(value, (list, tuple, set)): | ||
| return [_jsonable_default(v) for v in value] |
There was a problem hiding this comment.
set iteration in _jsonable_default is non-deterministic
When a ParamSpec.default or choices contains a set, _jsonable_default iterates it and returns a list, but sets are unordered in CPython 3.7+, so repeated calls may produce different orderings. Downstream consumers that compare serialized contracts (e.g., caching or hashing) may see spurious differences. Sorting the elements before building the list (or converting to a sorted list at the point where set defaults are declared) would make the output stable.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| def describe(self) -> StageContract: | ||
| return StageContract(cardinality="1:N fan-out", wrappable=False) |
There was a problem hiding this comment.
ManifestReader.describe() omits the audio_filepath write from its contract
ManifestReader decomposes into FilePartitioningStage + ManifestReaderStage, and the inner ManifestReaderStage.describe() correctly advertises writes=IOSpec(data_keys=["audio_filepath"], ...). But ManifestReader.describe() returns only StageContract(cardinality="1:N fan-out", wrappable=False) with no writes declared. An agent consuming the composite stage's contract won't know that audio_filepath is produced, so it cannot chain a downstream stage that reads that key using the static-discovery path. Since wrappable=False prevents wrapping, the agent must fall back to inspecting the decomposed stages manually.
1 participant
Summary
Makes the dataset initial-manifest stages agent-ready. Additive only.
datasets/fleurs/create_initial_manifest.py,datasets/readspeech/create_initial_manifest.py— subclassAgentReadyand adddescribe()/StageContract. These are source stages: they read a dataset layout and emitaudio_filepath+ metadata records, which the contract now declares. Manifest-creation logic unchanged.Notes for reviewers
Test plan
pytest tests/stages/audio/datasets -q