Remove dead document_grounded_sdg registration of GenselectAnswersStage - #13
Open
Anilreddy2309 wants to merge 1 commit into
Open
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 unreachableEvery
document-grounded-sdg*.yamlworkflow usesgym_genselect_answersinstead (registered separately innvflow/generic_stage/sdg/document_grounded/__init__.py), anddocument-grounded-sdg.yaml's own pipeline comment says it "replaces genselect_answers for DG-SDG". I couldn't find any config, test, or doc referencingfinance.document_grounded_sdg.genselect_answersspecifically —docs/recipes/finance/stages/template-based-sdg.mddocuments only thetemplate_based_sdgregistration 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_answerswith 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_sdgreturns 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 indocs/development/sdg/document_grounded/ADDING_A_DOMAIN.md.Test plan
ruff check/ruff format --checkpassfinance.template_based_sdg.genselect_answersstill resolves vianflow stage-infofinance.document_grounded_sdg.genselect_answersnow correctly 404s vianflow stage-info(it was never a valid path in any shipped config)pytest tests/suite passes (339 passed, 3 pre-existing skips) with no regressions🤖 Generated with Claude Code