Skip to content

feat(audio-segmentation): agent-ready speaker-separation + VAD (provenance fix)#2177

Open
shubhamNvidia wants to merge 3 commits into
NVIDIA-NeMo:mainfrom
shubhamNvidia:agent_pr/segmentation
Open

feat(audio-segmentation): agent-ready speaker-separation + VAD (provenance fix)#2177
shubhamNvidia wants to merge 3 commits into
NVIDIA-NeMo:mainfrom
shubhamNvidia:agent_pr/segmentation

Conversation

@shubhamNvidia

Copy link
Copy Markdown
Contributor

Summary

Makes the segmentation stages agent-ready and fixes speaker-separation provenance.

segmentation/speaker_separation.py (1:N fan-out) — provenance fix

  • Splits one multi-speaker input into one record per speaker. Previously each child inherited the parent audio_filepath (the full mixed file) and carried no source provenance. Now each child drops the parent file path and snapshots original_file (from original_file / audio_filepath) before emission, and carries a distinct speaker_id plus per-speaker waveform / diar_segments.

segmentation/vad_segmentation.py

  • Residency-aware VAD (accepts waveform or file), with fan-out (one task per segment) or nested (all segments under one key) modes, configurable keys, and describe().

Notes for reviewers

  • Backward compatible defaults; residency default "auto" prefers in-memory waveform.
  • Worth a look: _build_speaker_tasks in speaker_separation.py (which parent keys are dropped vs inherited).

Test plan

  • pytest tests/stages/audio/segmentation -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:52
@shubhamNvidia
shubhamNvidia requested review from suiyoubi and removed request for a team July 7, 2026 15:52
@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 the audio segmentation stages agent-ready by adding AgentReady mixin, describe() contracts, and configurable keys across VADSegmentationStage and SpeakerSeparationStage, and introduces two new infrastructure files (_agent_ready.py, _residency.py) that centralize audio-form negotiation and discovery contracts.

  • _agent_ready.py / _residency.py: New files establishing StageContract, IOSpec, Gates, ParamSpec, and the resolve_audio / resolve_audio_path helpers that replace ad-hoc waveform resolution in both segmentation stages.
  • SpeakerSeparationStage: Provenance fixed — child tasks now drop the parent audio_filepath and snapshot original_file before emission; residency-aware audio loading replaces resolve_waveform_from_item; _metadata and _stage_perf are correctly propagated to child tasks.
  • VADSegmentationStage: Adds nested/fan-out modes, configurable keys, keep_segment_waveform_in_task flag, residency-aware loading, and a describe() contract; fan-out mode with keep_segment_waveform_in_task=False leaves audio_filepath in child tasks without a guard (see inline comment).

Confidence Score: 4/5

Safe to merge with one fix: fan-out VAD segmentation with keep_segment_waveform_in_task=False silently passes audio_filepath to child tasks, causing downstream auto-mode stages to reload the full original file instead of the extracted segment.

The new infrastructure files and speaker-separation provenance fix are clean. The vad_segmentation fan-out path has a real correctness gap: stripping the waveform while leaving audio_filepath intact causes wrong data when any downstream stage resolves audio in auto mode — it will load the full recording for every segment instead of the sliced portion.

nemo_curator/stages/audio/segmentation/vad_segmentation.py — specifically _build_segment_item and the missing guard in post_init for fan-out + keep_segment_waveform_in_task=False.

Important Files Changed

Filename Overview
nemo_curator/stages/audio/_agent_ready.py New file: defines StageContract, IOSpec, Gates, ParamSpec, and AgentReady mixin; clean implementation with no issues found.
nemo_curator/stages/audio/_residency.py New file: centralizes audio resolution logic (waveform vs file, temp WAV materialisation); well-structured with appropriate fallback and cleanup helpers.
nemo_curator/stages/audio/common.py Adds AgentReady mixin and describe() to five existing stages; contracts are accurate and minimal.
nemo_curator/stages/audio/segmentation/speaker_separation.py Adds AgentReady + residency-aware audio resolution; provenance fix drops parent audio_filepath from child tasks. Hardcoded "original_file" key in describe() and runtime data (flagged in prior review thread).
nemo_curator/stages/audio/segmentation/vad_segmentation.py Adds AgentReady, configurable keys, nested/fan-out modes, and keep_segment_waveform_in_task. Fan-out mode with keep_segment_waveform_in_task=False inherits audio_filepath without warning, causing downstream auto-mode stages to reload the full file instead of the segment.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Agent
    participant VADSegmentationStage
    participant SpeakerSeparationStage
    participant resolve_audio

    Agent->>VADSegmentationStage: describe()
    VADSegmentationStage-->>Agent: "cardinality=1:N fan-out, reads_one_of=[waveform|file]"

    Agent->>SpeakerSeparationStage: describe()
    SpeakerSeparationStage-->>Agent: "cardinality=1:N fan-out, iteration_key=speaker_id_key"

    Agent->>VADSegmentationStage: process(task)
    VADSegmentationStage->>resolve_audio: (item, residency, keys)
    resolve_audio-->>VADSegmentationStage: (waveform, sample_rate) or None
    VADSegmentationStage->>VADSegmentationStage: _build_segment_item x N
    VADSegmentationStage-->>Agent: list[AudioTask] per segment

    Agent->>SpeakerSeparationStage: process(task)
    SpeakerSeparationStage->>resolve_audio: (item, residency, keys)
    resolve_audio-->>SpeakerSeparationStage: (waveform, sample_rate) or None
    SpeakerSeparationStage->>SpeakerSeparationStage: _build_speaker_tasks (drops audio_filepath, sets original_file)
    SpeakerSeparationStage-->>Agent: list[AudioTask] per speaker
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 VADSegmentationStage
    participant SpeakerSeparationStage
    participant resolve_audio

    Agent->>VADSegmentationStage: describe()
    VADSegmentationStage-->>Agent: "cardinality=1:N fan-out, reads_one_of=[waveform|file]"

    Agent->>SpeakerSeparationStage: describe()
    SpeakerSeparationStage-->>Agent: "cardinality=1:N fan-out, iteration_key=speaker_id_key"

    Agent->>VADSegmentationStage: process(task)
    VADSegmentationStage->>resolve_audio: (item, residency, keys)
    resolve_audio-->>VADSegmentationStage: (waveform, sample_rate) or None
    VADSegmentationStage->>VADSegmentationStage: _build_segment_item x N
    VADSegmentationStage-->>Agent: list[AudioTask] per segment

    Agent->>SpeakerSeparationStage: process(task)
    SpeakerSeparationStage->>resolve_audio: (item, residency, keys)
    resolve_audio-->>SpeakerSeparationStage: (waveform, sample_rate) or None
    SpeakerSeparationStage->>SpeakerSeparationStage: _build_speaker_tasks (drops audio_filepath, sets original_file)
    SpeakerSeparationStage-->>Agent: list[AudioTask] per speaker
Loading

Comments Outside Diff (1)

  1. nemo_curator/stages/audio/segmentation/vad_segmentation.py, line 968-989 (link)

    P1 keep_segment_waveform_in_task=False in fan-out mode silently causes downstream to reprocess the full audio file

    _build_segment_item does not exclude self.audio_filepath_key from the inherited keys, so every segment task keeps the parent's audio_filepath (which points at the full original recording). When keep_segment_waveform_in_task=False, there is no in-memory waveform. A downstream stage with input_residency="auto" will find no waveform key, fall back to audio_filepath, and load the full original file — not just the extracted segment boundaries — for every child task, silently producing wrong data.

    The __post_init__ guard fires only for nested=True and keep_segment_waveform_in_task=False, leaving the fan-out combination completely unguarded. Either drop self.audio_filepath_key from segment_data in fan-out mode when the waveform is stripped (mirroring what SpeakerSeparationStage._build_speaker_tasks does), or emit a warning analogous to the nested-mode one.

Reviews (2): Last reviewed commit: "fix(audio): residency-derived accepts/re..." | Re-trigger Greptile

writes=IOSpec(data_keys=writes, produces=produces),
cardinality="1:1 nested-list" if self.nested else "1:N fan-out",
cardinality_options=["fan_out", "nested"],
iteration_key=self.segments_key,

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 Wrong iteration_key in fan-out mode

iteration_key=self.segments_key is always emitted from describe(), but in fan-out mode (nested=False) no child task ever carries a "segments" key — each child has segment_num, start_ms, etc. An agent that uses this contract to identify which key distinguishes children will look for a non-existent key. The correct value for fan-out is self.segment_num_key; for nested it remains self.segments_key.

Comment on lines +160 to +170
return StageContract(
reads_one_of=[
IOSpec(data_keys=[self.waveform_key, self.sample_rate_key], accepts=["waveform"]),
IOSpec(data_keys=[self.audio_filepath_key], accepts=["file"]),
],
writes=IOSpec(data_keys=writes, produces=produces),
cardinality="1:1 nested-list" if self.nested else "1:N fan-out",
cardinality_options=["fan_out", "nested"],
iteration_key=self.segments_key,
gates=Gates(requires_gpu=self.resources.gpus > 0),
)

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 In fan-out mode the child tasks don't carry a "segments" key, so iteration_key should point at segment_num_key instead. Using segments_key here makes the contract inaccurate for agents.

Suggested change
return StageContract(
reads_one_of=[
IOSpec(data_keys=[self.waveform_key, self.sample_rate_key], accepts=["waveform"]),
IOSpec(data_keys=[self.audio_filepath_key], accepts=["file"]),
],
writes=IOSpec(data_keys=writes, produces=produces),
cardinality="1:1 nested-list" if self.nested else "1:N fan-out",
cardinality_options=["fan_out", "nested"],
iteration_key=self.segments_key,
gates=Gates(requires_gpu=self.resources.gpus > 0),
)
return StageContract(
reads_one_of=[
IOSpec(data_keys=[self.waveform_key, self.sample_rate_key], accepts=["waveform"]),
IOSpec(data_keys=[self.audio_filepath_key], accepts=["file"]),
],
writes=IOSpec(data_keys=writes, produces=produces),
cardinality="1:1 nested-list" if self.nested else "1:N fan-out",
cardinality_options=["fan_out", "nested"],
iteration_key=self.segments_key if self.nested else self.segment_num_key,
gates=Gates(requires_gpu=self.resources.gpus > 0),
)

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!

Comment on lines 148 to +262
@@ -178,21 +234,39 @@ def _build_speaker_tasks(
logger.debug(f"Skipping {speaker_id}: duration {result.duration:.2f}s < {self.min_duration}s")
continue
spk_waveform, spk_sr = _pydub_to_waveform_sr(result.audio)
# Drop the parent's file path(s) too: they point at the FULL
# multi-speaker file, so a file-preferring downstream stage would
# process the whole file per speaker instead of this speaker's
# extracted waveform. With the path gone, downstream resolves the
# per-speaker waveform (input_residency="auto") instead.
drop_keys = {
*self._INHERITED_DROP_KEYS,
self.waveform_key,
self.duration_key,
self.audio_filepath_key,
"audio_filepath",
}
speaker_data = {
**{k: v for k, v in item.items() if k not in self._INHERITED_DROP_KEYS},
"waveform": spk_waveform,
"sample_rate": spk_sr,
"speaker_id": speaker_id,
"num_speakers": num_speakers,
"duration": result.duration,
"diar_segments": result.diar_segments,
**{k: v for k, v in item.items() if k not in drop_keys},
self.waveform_key: spk_waveform,
self.sample_rate_key: spk_sr,
self.speaker_id_key: speaker_id,
self.num_speakers_key: num_speakers,
self.duration_key: result.duration,
self.diar_segments_key: result.diar_segments,
# Source identity must survive the audio_filepath drop above —
# TimestampMapper (and any provenance consumer) reads original_file.
"original_file": item.get("original_file")
or item.get(self.audio_filepath_key)
or item.get("audio_filepath")
or "unknown",

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 "original_file" key is hardcoded in SpeakerSeparationStage

VADSegmentationStage exposes original_file_key as a configurable dataclass field (default "original_file"), but SpeakerSeparationStage hardcodes the string "original_file" in both the describe() contract (data_keys=[..., "original_file"]) and the runtime data ("original_file": item.get("original_file") or ...). When these two stages are chained and the user sets a non-default original_file_key in VAD, the speaker-separation stage reads and writes a different key, silently breaking provenance for all downstream consumers.

Comment on lines +289 to 297
audio_result = resolve_audio(
item,
residency=self.input_residency, # type: ignore[arg-type]
audio_filepath_key=self.audio_filepath_key,
waveform_key=self.waveform_key,
sample_rate_key=self.sample_rate_key,
)
if audio_result is None:
return []

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 Silent drop when audio resolution fails

The old resolve_waveform_from_item logged an error before returning None. The new resolve_audio call returns None without any log, and the callsite just returns [] with no diagnostic. A task that silently disappears from the pipeline (missing waveform and non-existent file path) will be very hard to debug. Adding a log at the if audio_result is None guard (mirroring _resolve_audio in VADSegmentationStage) would preserve the previous observability contract.

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