Remove work-around for qdrant/qdrant-client#983

[dagardner-nv]

Description

• Invalid escape sequence produces SyntaxWarning on run qdrant/qdrant-client#983 has been fixed, this PR remove our work-around (added in PR Fix or silence warnings emitted by tests #305) for that issue.
• Rename test utility classes which are not test-suites, avoids a warning from pytest about not being able to collect tests.
• Assert that the aiq compatibility alias emits a deprecation warning, which also removes these from the warnings summary.

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.

Summary by CodeRabbit

• Chores
  • Simplified test infrastructure by removing deprecated fixtures and streamlining test configuration.
  • Refactored test utilities and improved internal test structure for better maintainability across testing modules.

[coderabbitai]

Walkthrough

Cleanup of test infrastructure: removed a deprecation-suppression fixture from conftest, refactored the compatibility test module cleanup, and renamed test doubles in middleware tests to private conventions with underscore prefixes throughout multiple test files.

Changes

Cohort / File(s)	Summary
Fixture and deprecation cleanup tests/conftest.py	Removed patched_async_memory_client autouse fixture that suppressed Pydantic deprecation warnings and monkeypatched MemoryClient._validate_api_key, along with related imports (warnings, PydanticDeprecatedSince20).
Compatibility test refactoring tests/nat/compat/test_compatibility_aliases.py	Added _remove_aiq_modules() helper to purge aiq-related modules from sys.modules. Updated autouse fixture to call cleanup before and after tests. Modified test_compatibility_aliases() to conditionally import modules based on use_nat_namespace flag, with deprecation warning expectations isolated to the non-nat branch.
Cache middleware test model renaming tests/nat/middleware/test_cache_middleware.py	Renamed test model classes TestInput → _TestInput and TestOutput → _TestOutput. Updated all references in FunctionMiddlewareContext instantiation, mock returns, and cache population throughout the test suite.
Middleware components test double renaming tests/nat/middleware/test_middleware_components.py	Renamed test double classes TestMiddlewareConfig → _TestMiddlewareConfig and TestMiddleware → _TestMiddleware. Updated all decorator usages, type annotations, isinstance() checks, registry lookups, fixture yields, and test data references.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

• Verify all references to renamed classes in test_cache_middleware.py and test_middleware_components.py are consistently updated (mechanical but broad scope)
• Confirm the new _remove_aiq_modules() helper in test_compatibility_aliases.py correctly handles module cleanup before and after tests
• Validate that deprecation warning expectations are properly isolated to the non-nat branch in the compatibility test

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: removing a work-around for a specific upstream issue, which aligns with the PR's primary objective.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/nat/middleware/test_cache_middleware.py (1)

284-285: Remove unused noqa directives.

Static analysis indicates these blanket noqa directives are no longer suppressing any warnings and should be removed.

Apply this diff:

-        middleware._cache[key1] = _TestOutput(result="Result 1")  # noqa
-        middleware._cache[key2] = _TestOutput(result="Result 2")  # noqa
+        middleware._cache[key1] = _TestOutput(result="Result 1")
+        middleware._cache[key2] = _TestOutput(result="Result 2")

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a69e42 and a405480.

📒 Files selected for processing (4)

• tests/conftest.py (0 hunks)
• tests/nat/compat/test_compatibility_aliases.py (2 hunks)
• tests/nat/middleware/test_cache_middleware.py (8 hunks)
• tests/nat/middleware/test_middleware_components.py (8 hunks)

💤 Files with no reviewable changes (1)

• tests/conftest.py

🧰 Additional context used

📓 Path-based instructions (2)

**/*

⚙️ CodeRabbit configuration file

**/*: # Code Review Instructions

• Ensure the code follows best practices and coding standards. - For Python code, follow
  PEP 20 and
  PEP 8 for style guidelines.
• Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
  Example:

  def my_function(param1: int, param2: str) -> bool:
      pass

• For Python exception handling, ensure proper stack trace preservation:
  • When re-raising exceptions: use bare raise statements to maintain the original stack trace,
    and use logger.error() (not logger.exception()) to avoid duplicate stack trace output.
  • When catching and logging exceptions without re-raising: always use logger.exception()
    to capture the full stack trace information.

Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any

words listed in the ci/vale/styles/config/vocabularies/nat/reject.txt file, words that might appear to be
spelling mistakes but are listed in the ci/vale/styles/config/vocabularies/nat/accept.txt file are OK.

Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,

and should contain an Apache License 2.0 header comment at the top of each file.

• Confirm that copyright years are up-to date whenever a file is changed.

Files:

• tests/nat/middleware/test_cache_middleware.py
• tests/nat/middleware/test_middleware_components.py
• tests/nat/compat/test_compatibility_aliases.py

tests/**/*.py

⚙️ CodeRabbit configuration file

tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using the test_ prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using the fixture_ prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass

Files:

• tests/nat/middleware/test_cache_middleware.py
• tests/nat/middleware/test_middleware_components.py
• tests/nat/compat/test_compatibility_aliases.py

🧬 Code graph analysis (1)

tests/nat/middleware/test_middleware_components.py (4)

src/nat/middleware/function_middleware.py (1)

• FunctionMiddleware (43-97)

src/nat/cli/register_workflow.py (1)

• register_middleware (221-257)

src/nat/cli/type_registry.py (4)

• register_middleware (562-577)
• GlobalTypeRegistry (1112-1133)
• get (1117-1118)
• get_middleware (579-595)

src/nat/builder/workflow_builder.py (6)

• get_middleware (1046-1061)
• get_middleware (1487-1489)
• add_middleware (1016-1043)
• add_middleware (1482-1484)
• get_middleware_config (1064-1079)
• get_middleware_config (1492-1494)

🪛 Ruff (0.14.5)

tests/nat/middleware/test_cache_middleware.py

284-284: Unused blanket noqa directive

Remove unused noqa directive

(RUF100)

---

285-285: Unused blanket noqa directive

Remove unused noqa directive

(RUF100)

tests/nat/middleware/test_middleware_components.py

60-60: Unused function argument: builder

(ARG001)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: CI Pipeline / Check

🔇 Additional comments (6)

tests/nat/compat/test_compatibility_aliases.py (3)

24-28: LGTM: Well-structured helper function.

The extraction of module cleanup logic into _remove_aiq_modules() improves code clarity and reusability. The iteration over a copy of sys.modules.keys() safely handles dictionary modification during iteration.

---

31-41: LGTM: Proper cleanup before and after tests.

The fixture correctly invokes _remove_aiq_modules() both before yielding (line 37) and in teardown (line 41), ensuring a clean test environment for each test run.

---

106-119: LGTM: Correct deprecation warning handling.

The branching logic properly validates deprecation behavior:

• When use_nat_namespace=True: imports nat modules directly without expecting warnings
• When use_nat_namespace=False: wraps aiq imports in pytest.deprecated_call() to assert deprecation warnings are emitted

This aligns with the PR objective to verify aiq compatibility aliases emit deprecation warnings.

tests/nat/middleware/test_cache_middleware.py (1)

32-40: LGTM: Renaming prevents pytest collection warnings.

Prefixing with underscore (_TestInput, _TestOutput) correctly signals these are test doubles rather than test suites, avoiding PytestCollectionWarning about collecting non-test classes.

tests/nat/middleware/test_middleware_components.py (2)

33-46: LGTM: Renaming prevents pytest collection warnings.

Prefixing with underscore (_TestMiddlewareConfig, _TestMiddleware) correctly signals these are test doubles rather than test suites, consistent with the renaming in test_cache_middleware.py.

---

59-61: The unused builder parameter is likely required by the framework.

While static analysis flags builder as unused, the @register_middleware decorator expects a build function with signature (config, builder) as shown in the relevant code snippets. The parameter cannot be removed without breaking the registration contract, even though this simple test middleware doesn't need access to the builder.

[dagardner-nv]

/merge