Skip to content

Remove dead document_grounded_sdg registration of GenselectAnswersStage - #13

Open
Anilreddy2309 wants to merge 1 commit into
NVIDIA:mainfrom
Anilreddy2309:cleanup/remove-dead-genselect-registration
Open

Remove dead document_grounded_sdg registration of GenselectAnswersStage#13
Anilreddy2309 wants to merge 1 commit into
NVIDIA:mainfrom
Anilreddy2309:cleanup/remove-dead-genselect-registration

Conversation

@Anilreddy2309

Copy link
Copy Markdown

Summary

GenselectAnswersStage (nvflow/recipes/finance/stages/sdg/genselect_answers.py) was registered twice:

  • recipe=finance, workflow=template_based_sdg, stage=genselect_answers — actively used (template-based-sdg*.yaml)
  • recipe=finance, workflow=document_grounded_sdg, stage=genselect_answers — appears unreachable

Every document-grounded-sdg*.yaml workflow uses gym_genselect_answers instead (registered separately in nvflow/generic_stage/sdg/document_grounded/__init__.py), and document-grounded-sdg.yaml's own pipeline comment says it "replaces genselect_answers for DG-SDG". I couldn't find any config, test, or doc referencing finance.document_grounded_sdg.genselect_answers specifically — docs/recipes/finance/stages/template-based-sdg.md documents only the template_based_sdg registration for this stage.

This PR removes the second, apparently-dead @StageRegistry.register(...) decorator.

Flagging for a maintainer to confirm rather than assuming: if this was intentionally left registered for backward compatibility (e.g. an external caller invoking nflow run genselect_answers with an older document-grounded-sdg config), happy to close this instead and add a comment explaining why it's kept. I didn't find evidence of that in the repo, but wanted to call it out explicitly since it's a behavior-affecting removal rather than pure cleanup.

Verification

nflow list-stages --recipe finance --workflow document_grounded_sdg returns the same 7 stages before and after this change (dg_sdg_preprocess, generate_verified_questions, generate_answers, gym_genselect_answers, evaluate_answers, aggregate_answers, dgsdg_post_process) — matching the documented pipeline in docs/development/sdg/document_grounded/ADDING_A_DOMAIN.md.

Test plan

  • ruff check / ruff format --check pass
  • Manually confirmed finance.template_based_sdg.genselect_answers still resolves via nflow stage-info
  • Manually confirmed finance.document_grounded_sdg.genselect_answers now correctly 404s via nflow stage-info (it was never a valid path in any shipped config)
  • Full pytest tests/ suite passes (339 passed, 3 pre-existing skips) with no regressions

🤖 Generated with Claude Code

GenselectAnswersStage was registered twice: once for
recipe=finance/workflow=template_based_sdg (still used, see
template-based-sdg*.yaml), and once for
recipe=finance/workflow=document_grounded_sdg. The second registration
is unreachable -- every document-grounded-sdg workflow YAML uses
gym_genselect_answers instead (a separate stage registered in
nvflow/generic_stage/sdg/document_grounded/__init__.py), and
document-grounded-sdg.yaml's own comment says gym_genselect_answers
"replaces genselect_answers for DG-SDG". No config anywhere references
finance.document_grounded_sdg.genselect_answers, and the stage's own
docs (docs/recipes/finance/stages/template-based-sdg.md) only document
the template_based_sdg registration.

Verified via `nflow list-stages --recipe finance --workflow
document_grounded_sdg`: the 7 registered stages are unchanged before
and after (dg_sdg_preprocess, generate_verified_questions,
generate_answers, gym_genselect_answers, evaluate_answers,
aggregate_answers, dgsdg_post_process), matching the documented
pipeline in docs/development/sdg/document_grounded/ADDING_A_DOMAIN.md.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Anil Balireddy <anilbalireddi@gmail.com>
@Anilreddy2309
Anilreddy2309 requested a review from a team September 11, 2026 16:58
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