Skip to content

fix(library): missing "global $relevant_chunks" declaration in hf_classifier retrieval flows - #2201

Open
ekam-walia wants to merge 4 commits into
NVIDIA-NeMo:developfrom
ekam-walia:missing-global-relevant-chunk
Open

fix(library): missing "global $relevant_chunks" declaration in hf_classifier retrieval flows#2201
ekam-walia wants to merge 4 commits into
NVIDIA-NeMo:developfrom
ekam-walia:missing-global-relevant-chunk

Conversation

@ekam-walia

@ekam-walia ekam-walia commented Jul 22, 2026

Copy link
Copy Markdown

Description

Added global $relevant_chunks before the reassignment in both flow files:

  1. nemoguardrails/library/hf_classifier/flows.co
  2. nemoguardrails/library/hf_classifier/flows.v1.co

Related Issue(s)

Every PR must link to a triaged issue assigned to the PR author.

Verification

AI Assistance

  • [ x] No AI tools were used.

Checklist

  • [ x] I've read the CONTRIBUTING guidelines.
  • [ x] This PR links to a triaged issue assigned to me.
  • [ x] My PR title follows the project commit convention.
  • I've updated the documentation if applicable.
  • [ x] I've added tests if applicable.
  • [ x] I've noted any verification beyond CI and any checks I couldn't run.
  • I did not update generated changelog files manually.
  • [ x] I addressed all CodeRabbit, Greptile, and other review comments, or replied with why no change is needed.
  • @mentions of the person or team responsible for reviewing proposed changes.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed retrieval classification so relevant content is correctly preserved and available to calling flows.
    • Ensured existing content remains unchanged when no transformation occurs or processing is blocked.
    • Improved consistency across classifier flow versions and repeated classifier calls.
  • Tests

    • Added comprehensive coverage for retrieval propagation, blocked responses, empty results, and classifier configurations.

@github-actions github-actions Bot added size: M status: needs triage New issues that have not yet been reviewed or categorized. needs: signing labels Jul 22, 2026
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

PR merge guidance

@ekam-walia thanks for the PR. GitHub is currently blocking merge for one or more repository requirements:

  • 4 commits do not have a verified signature (0b49f39, c10d0b5, bf12276, c552210). Please sign the commits and force-push the updated branch.

Relevant guide:

@ekam-walia

Copy link
Copy Markdown
Author

cc @Pouyanpi

@Pouyanpi Pouyanpi changed the title fix of bug: missing "global $relevant_chunks" declaration in hf_classifier retrieval flows fix(library): missing "global $relevant_chunks" declaration in hf_classifier retrieval flows Jul 22, 2026
@ekam-walia
ekam-walia force-pushed the missing-global-relevant-chunk branch from 3a0d450 to c10d0b5 Compare July 22, 2026 17:16
@ekam-walia

Copy link
Copy Markdown
Author

@Pouyanpi can you please review, if the code is making sense

@ekam-walia

Copy link
Copy Markdown
Author

@Pouyanpi any updates on this PR

@Pouyanpi

Copy link
Copy Markdown
Collaborator

@ekam-walia I've reviewed your PR but you've not addressed my review comments. Please take a look

@Pouyanpi Pouyanpi added status: triaged Triaged by a maintainer; eligible for automated review (CodeRabbit/Greptile). and removed status: needs triage New issues that have not yet been reviewed or categorized. labels Jul 27, 2026
@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes retrieval-chunk propagation across both Colang dialects.

  • Declares $relevant_chunks global before transformed assignment in the Colang 2.x flow.
  • Uses Colang 1.0 shared-context assignment without the unsupported global statement or a discarded subflow return value.
  • Adds regression coverage for transformed, unchanged, empty, and blocked retrieval results.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the Colang 1.0 flow now relies on its shared context without unsupported syntax or a discarded return value, while the Colang 2.x flow explicitly updates the global retrieval context.

Important Files Changed

Filename Overview
nemoguardrails/library/hf_classifier/flows.co Adds the valid Colang 2.x global declaration so transformed retrieval chunks propagate to the calling context.
nemoguardrails/library/hf_classifier/flows.v1.co Preserves direct shared-context assignment while removing the unsupported global statement and avoiding reliance on a subflow return value.
tests/test_hf_classifier_retrieval_global.py Adds regression scenarios covering transformed chunks, unchanged chunks, empty transformations, and blocked retrieval checks.

Reviews (3): Last reviewed commit: "." | Re-trigger Greptile

Comment thread nemoguardrails/library/hf_classifier/flows.v1.co Outdated
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The HF classifier retrieval flows now propagate transformed relevant_chunks through the shared context. New tests cover v1 and original flows, caller visibility, blocked responses, empty transforms, repeated calls, and explicit classifier parameters.

Changes

HF classifier retrieval propagation

Layer / File(s) Summary
Update retrieval flow scope
nemoguardrails/library/hf_classifier/flows.co, nemoguardrails/library/hf_classifier/flows.v1.co
Both retrieval flows declare relevant_chunks as global before assigning transformed values.
Validate propagation behavior
tests/test_hf_classifier_retrieval_global.py
Adds mocked classifier tests covering transformed, empty, blocked, repeated, caller-flow, original-flow, and explicit-classifier scenarios.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main fix: adding the missing global declaration in both hf_classifier retrieval flows.
Linked Issues check ✅ Passed The PR updates both retrieval flows and adds tests showing transformed relevant_chunks propagates as required by #2172.
Out of Scope Changes check ✅ Passed The changes stay focused on the reported bug fix and the matching test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Test Results For Major Changes ✅ Passed PASS: This is a minor two-line scope fix in the flow files, so the empty Verification section is acceptable under the check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_hf_classifier_retrieval_global.py`:
- Around line 1-13: Remove the manually added copyright and Apache license
header from tests/test_hf_classifier_retrieval_global.py, leaving pre-commit to
insert the repository-standard header automatically.
- Line 36: Remove the newly added explanatory # comments throughout the test
cases in tests/test_hf_classifier_retrieval_global.py, including the listed
ranges, while preserving all test code and behavior unchanged.
- Around line 202-240: Update
test_hf_classifier_v1_retrieval_blocked_with_exceptions to exercise the
retrieval flow and changed retrieval scope rather than registering and executing
hf_classifier_check_input. Either move the test into the input-flow test area if
it is intended to cover input behavior, or register and invoke the retrieval
action/flow while preserving the assertion that blocked processing leaves
relevant_chunks unchanged.
- Around line 163-200: Update
test_hf_classifier_v1_retrieval_with_empty_transform_text so
MockHfClassifierResponse.transform_text includes an empty relevant_chunks value,
matching the v1 flow’s transform_text["relevant_chunks"] lookup. Assert both the
original_chunks context and relevant_chunks context, verifying the original
remains unchanged while the transformed value is propagated.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: baf1210b-6c7a-4c74-848f-478752181795

📥 Commits

Reviewing files that changed from the base of the PR and between b237e39 and c10d0b5.

📒 Files selected for processing (3)
  • nemoguardrails/library/hf_classifier/flows.co
  • nemoguardrails/library/hf_classifier/flows.v1.co
  • tests/test_hf_classifier_retrieval_global.py

Comment thread tests/test_hf_classifier_retrieval_global.py Outdated
Comment thread tests/test_hf_classifier_retrieval_global.py Outdated
Comment thread tests/test_hf_classifier_retrieval_global.py Outdated
Comment thread tests/test_hf_classifier_retrieval_global.py Outdated
Comment thread nemoguardrails/library/hf_classifier/flows.v1.co Outdated
@ekam-walia

Copy link
Copy Markdown
Author

@Pouyanpi ptal made the reqd changes

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

Labels

needs: signing size: M status: triaged Triaged by a maintainer; eligible for automated review (CodeRabbit/Greptile).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: missing "global $relevant_chunks" declaration in hf_classifier retrieval flows

2 participants