Skip to content

feat(core): expose read-only views of function group filters - #2147

Draft
DABH wants to merge 1 commit into
NVIDIA:developfrom
DABH:function-group-filter-introspection
Draft

feat(core): expose read-only views of function group filters#2147
DABH wants to merge 1 commit into
NVIDIA:developfrom
DABH:function-group-filter-introspection

Conversation

@DABH

@DABH DABH commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Description

FunctionGroup is part of the public plugin-authoring contract: third-party plugin packages import it from the stable nat.plugin_api facade (per docs/source/extend/third-party-plugins.md, symbols exported from nat.plugin_api are the public contract for external plugin packages) and build provider tool groups through register_function_group. A group's composition can be shaped by two kinds of filter callbacks: a group-level filter_fn (constructor argument, or set_filter_fn) and per-function filter callbacks (add_function(..., filter_fn=...), or set_per_function_filter_fn). Today both are write-only from the outside: a plugin package that wants to validate, document, or instrument how a group is composed (for example, asserting in its own tests that the expected filters were wired up, or logging which functions are dynamically gated) has to reach into the private _filter_fn and _per_function_filter_fn attributes, which is exactly the kind of implementation-module reliance the third-party plugin guide tells authors to avoid.

This change adds two minimal read-only properties to FunctionGroup, mirroring the existing instance_name and middleware properties:

  • filter_fn returns the configured group-level filter callback, or None when no group-level filter has been set.
  • per_function_filter_fns returns an immutable types.MappingProxyType view of the per-function filter callbacks keyed by function name. The view rejects mutation with TypeError and stays live: filter callbacks registered later (through add_function or set_per_function_filter_fn) appear in a previously obtained view.

The change is purely additive: no existing behavior changes, and no new module-level symbols are added to nat.plugin_api (the properties ride along on the already-exported FunctionGroup class, so the pinned export test and the plugin API surface documentation are unaffected).

No tracking issue exists for this yet; happy to file one if the team prefers.

Testing

  • uv run pytest packages/nvidia_nat_core/tests/nat/builder/test_function_group.py — 28 passed (includes two new tests covering groups constructed with and without filters, filters set after construction, read-only enforcement on the mapping view, and the view reflecting later additions).
  • uv run pytest packages/nvidia_nat_core/tests/nat/builder — 319 passed.
  • uv run pre-commit run yapf --files packages/nvidia_nat_core/src/nat/builder/function.py packages/nvidia_nat_core/tests/nat/builder/test_function_group.py — passed.
  • uv run pre-commit run ruff-check --files packages/nvidia_nat_core/src/nat/builder/function.py packages/nvidia_nat_core/tests/nat/builder/test_function_group.py — passed.
  • uv run python ci/scripts/copyright.py --verify-apache-v2 — passed.

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
    • Any contribution which contains commits that are not Signed-Off will not be accepted.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

Signed-off-by: David Hyde <DABH@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 28, 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.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 358a2dc9-fc58-44e2-8262-6202ed7b02f0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@willkill07

Copy link
Copy Markdown
Member

a plugin package that wants to validate, document, or instrument how a group is composed (for example, asserting in its own tests that the expected filters were wired up, or logging which functions are dynamically gated) has to reach into the private _filter_fn and _per_function_filter_fn attributes, which is exactly the kind of implementation-module reliance the third-party plugin guide tells authors to avoid.

I get your point, but at the same time, we intentionally don't want to expose filter_fn or per_function_filter_fn, even as a read-only property. Since it's not necessary for the runtime behavior and is only for testing, I don't see why manual inspection is inappropriate.

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.

2 participants