Skip to content

feat(audio-advanced): agent-ready AudioDataFilter composite pipeline#2181

Open
shubhamNvidia wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
shubhamNvidia:agent_pr/advanced_pipelines
Open

feat(audio-advanced): agent-ready AudioDataFilter composite pipeline#2181
shubhamNvidia wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
shubhamNvidia:agent_pr/advanced_pipelines

Conversation

@shubhamNvidia

Copy link
Copy Markdown
Contributor

Summary

Makes the composite AudioDataFilterStage agent-ready. Additive only.

  • advanced_pipelines/audio_data_filter/audio_data_filter.py — the composite (which decomposes into the underlying filter stages) now subclasses AgentReady and exposes describe() / StageContract. Decomposition and runtime behavior unchanged.

Notes for reviewers

  • Additive, backward compatible.

Test plan

  • pytest tests/stages/audio/advanced_pipelines -q

…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).
@shubhamNvidia shubhamNvidia requested a review from a team as a code owner July 7, 2026 15:55
@shubhamNvidia shubhamNvidia requested review from meatybobby and removed request for a team July 7, 2026 15:55
@copy-pr-bot

copy-pr-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes AudioDataFilterStage and several supporting stages in nemo_curator/stages/audio/ agent-discoverable by adding the new AgentReady mixin and implementing describe() → StageContract. A companion _residency.py module is also introduced to centralise waveform/path resolution logic.

  • _agent_ready.py defines the full discovery contract vocabulary (StageContract, IOSpec, Gates, ParamSpec, etc.) plus the AgentReady mixin with describe() / describe_static().
  • common.py adds AgentReady and describe() to five existing stages; PreserveByValueStage.describe() omits batch_only=True despite the class-level BATCH_ONLY = True and a process() that raises immediately.
  • _residency.py introduces resolve_audio, resolve_audio_path, and cleanup_temp_files; the temp file created by mkstemp inside resolve_audio_path is not cleaned up if sf.write() fails, and _as_soundfile_array may return an incorrectly-shaped array for clips where channels >= samples.

Confidence Score: 3/5

Safe to merge after addressing the batch_only contract mismatch in PreserveByValueStage and the temp-file leak in _residency.py; AudioDataFilterStage itself is unchanged at runtime.

PreserveByValueStage.describe() returns a contract that says process() is callable, but calling it raises NotImplementedError — any agent or tool that consults describe() directly (rather than going through the registry) will act on wrong information. The _residency.py temp-file leak on sf.write() failure and the _as_soundfile_array shape fall-through for short clips are real defects, though both are narrow error paths unlikely to hit in typical usage. The core AudioDataFilterStage decomposition logic is untouched and the additive nature of the change limits blast radius.

nemo_curator/stages/audio/common.py (PreserveByValueStage.describe) and nemo_curator/stages/audio/_residency.py (resolve_audio_path and _as_soundfile_array)

Important Files Changed

Filename Overview
nemo_curator/stages/audio/_agent_ready.py New file introducing the AgentReady mixin and all supporting dataclasses (StageContract, IOSpec, Gates, etc.) for agent-facing discovery contracts. Well-structured; JSON-safe helpers are careful about non-serializable objects. No issues found.
nemo_curator/stages/audio/_residency.py New utility module for audio path/waveform resolution. Two issues: temp file leaked on sf.write() failure, and _as_soundfile_array falls through with wrong shape when channels >= samples.
nemo_curator/stages/audio/common.py Adds AgentReady mixin and describe() to five existing stages. PreserveByValueStage.describe() omits batch_only=True despite BATCH_ONLY=True and a process() that raises, producing a misleading contract for callers who skip the registry.
nemo_curator/stages/audio/advanced_pipelines/audio_data_filter/audio_data_filter.py Adds AgentReady mixin and a minimal describe() returning StageContract(wrappable=False) to AudioDataFilterStage. Decomposition logic and runtime behaviour unchanged.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Agent
    participant Stage as AudioDataFilterStage / other AgentReady stage
    participant describe as stage.describe()
    participant registry as _agent_registry.build_contract()

    Agent->>Stage: stage.describe()
    Stage->>describe: "returns StageContract(wrappable=False, ...)"
    describe-->>Agent: "StageContract (batch_only=False by default)"
    Note over Agent,Stage: PreserveByValueStage: BATCH_ONLY=True not reflected here

    Agent->>registry: build_contract(stage)
    registry->>Stage: reads cls.BATCH_ONLY
    registry-->>Agent: "StageContract (batch_only=True correctly set)"

    Agent->>Stage: stage.process(task)
    Note over Stage: PreserveByValueStage.process() raises NotImplementedError!
    Agent->>Stage: stage.process_batch(tasks)
    Stage-->>Agent: list[AudioTask]
Loading
%%{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 AudioDataFilterStage / other AgentReady stage
    participant describe as stage.describe()
    participant registry as _agent_registry.build_contract()

    Agent->>Stage: stage.describe()
    Stage->>describe: "returns StageContract(wrappable=False, ...)"
    describe-->>Agent: "StageContract (batch_only=False by default)"
    Note over Agent,Stage: PreserveByValueStage: BATCH_ONLY=True not reflected here

    Agent->>registry: build_contract(stage)
    registry->>Stage: reads cls.BATCH_ONLY
    registry-->>Agent: "StageContract (batch_only=True correctly set)"

    Agent->>Stage: stage.process(task)
    Note over Stage: PreserveByValueStage.process() raises NotImplementedError!
    Agent->>Stage: stage.process_batch(tasks)
    Stage-->>Agent: list[AudioTask]
Loading

Reviews (1): Last reviewed commit: "feat(audio-advanced_pipelines): make adv..." | Re-trigger Greptile

Comment on lines +119 to +124
def describe(self) -> StageContract:
return StageContract(
reads=IOSpec(data_keys=[self.input_value_key]),
writes=IOSpec(data_keys=[self.input_value_key]),
cardinality="filter",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 PreserveByValueStage.describe() returns StageContract with batch_only at its default of False, yet BATCH_ONLY = True is set on the class and process() immediately raises NotImplementedError. Any agent or tool that calls stage.describe() directly (instead of going through the registry's build_contract) will receive a contract that incorrectly advertises process() as callable, which would raise at runtime. The fix is to propagate the class flag into the returned contract.

Suggested change
def describe(self) -> StageContract:
return StageContract(
reads=IOSpec(data_keys=[self.input_value_key]),
writes=IOSpec(data_keys=[self.input_value_key]),
cardinality="filter",
)
def describe(self) -> StageContract:
return StageContract(
reads=IOSpec(data_keys=[self.input_value_key]),
writes=IOSpec(data_keys=[self.input_value_key]),
cardinality="filter",
batch_only=True,
)

Comment on lines +132 to +137
fd, tmp = tempfile.mkstemp(suffix=".wav", dir=temp_dir)
os.close(fd)
sf.write(tmp, _as_soundfile_array(waveform), int(sample_rate))
if register_temp is not None:
register_temp.append(tmp)
return tmp

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 If sf.write() raises (e.g. the waveform dtype is unsupported, or the array shape is wrong after _as_soundfile_array), the temp file created by mkstemp is left on disk but never appended to register_temp. The caller therefore never learns about it and cleanup_temp_files will not remove it. Wrapping the write in a try/except and cleaning up on failure closes the leak.

Suggested change
fd, tmp = tempfile.mkstemp(suffix=".wav", dir=temp_dir)
os.close(fd)
sf.write(tmp, _as_soundfile_array(waveform), int(sample_rate))
if register_temp is not None:
register_temp.append(tmp)
return tmp
fd, tmp = tempfile.mkstemp(suffix=".wav", dir=temp_dir)
os.close(fd)
try:
sf.write(tmp, _as_soundfile_array(waveform), int(sample_rate))
except Exception:
with contextlib.suppress(OSError):
os.remove(tmp)
raise
if register_temp is not None:
register_temp.append(tmp)
return tmp

Comment on lines +76 to +81
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 _as_soundfile_array falls through to return waveform when channels >= samples (i.e. a 2-D array whose first dimension is not smaller than the second). SoundFile expects audio in (samples, channels) shape for multi-channel data, so returning the un-transposed (channels, samples) array in that branch would write audio with channels and samples swapped, producing a corrupt WAV. In practice audio files have far more samples than channels, but a very short clip (e.g. a 1-sample test fixture) would silently trigger this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant