feat(knowledge): add Azure AI Search backend#308
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughAdds Azure AI Search as a knowledge-layer backend with configuration and packaging support, hybrid retrieval, ingestion and lifecycle operations, compliance tests, Web API configuration, and operational documentation. ChangesAzure AI Search Backend
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant AzureAISearchIngestor
participant SearchClient
participant AzureAISearchRetriever
Caller->>AzureAISearchIngestor: submit ingestion job
AzureAISearchIngestor->>SearchClient: upload embedded chunks
SearchClient-->>AzureAISearchIngestor: return upload results
Caller->>AzureAISearchRetriever: retrieve query
AzureAISearchRetriever->>SearchClient: execute scoped hybrid search
SearchClient-->>AzureAISearchRetriever: return matching chunks
AzureAISearchRetriever-->>Caller: return normalized results
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
AjayThorve
requested review from
AjayThorve and
KyleZheng1284
and removed request for
KyleZheng1284
|
/ok to test 1232f19 |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/source/customization/configuration-reference.md`:
- Around line 279-291: The configuration table currently shows
`azure_search_auth_mode` defaulting to `Auto`, which does not match the actual
`KnowledgeRetrievalConfig` behavior and can mislead users copying YAML. Update
the wording in the `configuration-reference.md` table entry for
`azure_search_auth_mode` to reflect the real default/auth selection logic, and
make it clear that only `managed_identity` and `api_key` are valid modes rather
than presenting `Auto` as a user-settable default.
In `@docs/source/customization/knowledge-layer.md`:
- Around line 159-189: The Azure AI Search example in knowledge-layer.md needs
blank lines separating the prose from the fenced YAML block to satisfy
markdownlint MD031. Update the section around the Azure AI Search example so the
fence is surrounded by empty lines, keeping the surrounding explanatory text and
the code block in the same place.
In `@sources/knowledge_layer/src/azure_ai_search/adapter.py`:
- Around line 1013-1020: The `_collection_info` helper is doing an expensive
full index walk by calling `list_files(name)` just to compute `file_count`,
which makes `list_collections` scale poorly. Update `_collection_info` in the
Azure AI Search adapter to avoid enumerating all documents for metadata; derive
`file_count` from cheaper indexed data such as a `file_id` facet or stored
marker metadata, and keep `get_document_count()` only for chunk counts. Leave
the full document iteration in `list_files` for explicit file listing only.
- Around line 492-493: The Azure AI Search adapter is forwarding
filters["$filter"] from retrieve() directly into client.search() without any
validation, so the filtering path in the adapter needs to constrain or sanitize
that OData before use. Update the filter handling near the
search_params["filter"] assignment to validate against an allowlist of supported
fields/operators, or otherwise reject unsafe expressions before passing them
onward. Keep the logic localized in the retrieve/search_params construction flow
so the existing filters dict handling remains intact.
In `@sources/knowledge_layer/src/register.py`:
- Around line 42-45: _update _url_from_env in register.py so an explicitly empty
environment value is treated like “unset” and does not override the fallback
default. Adjust the logic in _url_from_env to distinguish between missing and
empty values, and make sure callers like embed_base_url still receive the
intended default URL when AIQ_EMBED_BASE_URL is blank. Preserve the current
behavior for optional settings such as azure_search_endpoint while fixing the
fallback path used by non-optional URL fields.
In `@tests/knowledge_layer_tests/test_azure_ai_search.py`:
- Around line 276-294: The test data in
test_setup_backend_preserves_secrets_and_prefix is being flagged by
detect-secrets because of the inline api key literals. Add an inline pragma:
allowlist secret to the specific placeholder secret assignments in this test so
the scanner ignores them, while keeping the assertions that _setup_backend
preserves SecretStr handling and the index_prefix value.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: f0dc80cd-5107-4735-b4af-4c0152a48533
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
docs/source/customization/configuration-reference.mddocs/source/customization/knowledge-layer.mddocs/source/examples/azure-ai-search.mddocs/source/examples/index.mddocs/source/index.mdsources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mdsources/knowledge_layer/README.mdsources/knowledge_layer/pyproject.tomlsources/knowledge_layer/src/__init__.pysources/knowledge_layer/src/azure_ai_search/README.mdsources/knowledge_layer/src/azure_ai_search/__init__.pysources/knowledge_layer/src/azure_ai_search/adapter.pysources/knowledge_layer/src/register.pytests/knowledge_layer_tests/run_adapter_compliance.pytests/knowledge_layer_tests/test_azure_ai_search.py
📜 Review details
⚠️ CI failures not shown inline (4)
GitHub Actions: AIQ CI / 0_Lint and Hooks.txt: docs(knowledge): document Azure Search managed identity roles
Conclusion: failure
##[group]Run . .venv/bin/activate
�[36;1m. .venv/bin/activate�[0m
�[36;1mSKIP=ruff-check,ruff-format,pytest,helm-lint pre-commit run --all-files�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.13.14/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
[WARNING] hook id `helm-lint` uses deprecated stage names (push) which will be removed in a future version. run: `pre-commit migrate-config` to automatically fix this.
[WARNING] hook id `pytest` uses deprecated stage names (push) which will be removed in a future version. run: `pre-commit migrate-config` to automatically fix this.
[INFO] Initializing environment for https://github.com/astral-sh/ruff-pre-commit.
[INFO] Initializing environment for https://github.com/astral-sh/uv-pre-commit.
[INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks.
[WARNING] repo `https://github.com/pre-commit/pre-commit-hooks` uses deprecated stage names (commit, push) which will be removed in a future version. Hint: often `pre-commit autoupdate --repo https://github.com/pre-commit/pre-commit-hooks` will fix this. if it does not -- consider reporting an issue to that repo.
[INFO] Initializing environment for https://github.com/Yelp/detect-secrets.
[INFO] Initializing environment for local:pyyaml==6.0.3.
[INFO] Initializing environment for https://github.com/tcort/markdown-link-check.
[WARNING] repo `https://github.com/tcort/markdown-link-check` uses deprecated stage names (commit, push) which will be removed in a future version. Hint: often `pre-commit autoupdate --repo https://github.com/tcort/markdown-link-check` wi...
GitHub Actions: AIQ CI / Script Validation: docs(knowledge): document Azure Search managed identity roles
Conclusion: failure
##[group]Run . .venv/bin/activate
�[36;1m. .venv/bin/activate�[0m
�[36;1mchmod +x ci/scripts/test_scripts.sh�[0m
�[36;1mci/scripts/test_scripts.sh --skip-setup�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.13.14/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
================================================
AI-Q Blueprint - Script Tests
================================================
Repository: /home/runner/work/aiq/aiq
Scripts: /home/runner/work/aiq/aiq/scripts
============================================
Testing Bash Syntax
============================================
�[0;32m✅ PASS�[0m: dev.sh - valid bash syntax
�[0;32m✅ PASS�[0m: setup.sh - valid bash syntax
�[0;32m✅ PASS�[0m: setup_openshell.sh - valid bash syntax
�[0;32m✅ PASS�[0m: start_as_skill.sh - valid bash syntax
�[0;32m✅ PASS�[0m: start_cli.sh - valid bash syntax
�[0;32m✅ PASS�[0m: start_e2e.sh - valid bash syntax
�[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh - valid bash syntax
�[1;33m⏭️ SKIP�[0m: setup.sh - skipped (--skip-setup flag)
============================================
Testing --help Flags
============================================
�[0;32m✅ PASS�[0m: start_cli.sh --help
�[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh --help
============================================
Testing Virtual Environment Checks
============================================
�[0;32m✅ PASS�[0m: start_cli.sh - venv check works
�[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh - venv check works
============================================
Testing Pytest Integration
=====================...
GitHub Actions: AIQ CI / Lint and Hooks: docs(knowledge): document Azure Search managed identity roles
Conclusion: failure
##[group]Run . .venv/bin/activate
�[36;1m. .venv/bin/activate�[0m
�[36;1mSKIP=ruff-check,ruff-format,pytest,helm-lint pre-commit run --all-files�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.13.14/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
[WARNING] hook id `helm-lint` uses deprecated stage names (push) which will be removed in a future version. run: `pre-commit migrate-config` to automatically fix this.
[WARNING] hook id `pytest` uses deprecated stage names (push) which will be removed in a future version. run: `pre-commit migrate-config` to automatically fix this.
[INFO] Initializing environment for https://github.com/astral-sh/ruff-pre-commit.
[INFO] Initializing environment for https://github.com/astral-sh/uv-pre-commit.
[INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks.
[WARNING] repo `https://github.com/pre-commit/pre-commit-hooks` uses deprecated stage names (commit, push) which will be removed in a future version. Hint: often `pre-commit autoupdate --repo https://github.com/pre-commit/pre-commit-hooks` will fix this. if it does not -- consider reporting an issue to that repo.
[INFO] Initializing environment for https://github.com/Yelp/detect-secrets.
[INFO] Initializing environment for local:pyyaml==6.0.3.
[INFO] Initializing environment for https://github.com/tcort/markdown-link-check.
[WARNING] repo `https://github.com/tcort/markdown-link-check` uses deprecated stage names (commit, push) which will be removed in a future version. Hint: often `pre-commit autoupdate --repo https://github.com/tcort/markdown-link-check` wi...
GitHub Actions: AIQ CI / 3_Script Validation.txt: docs(knowledge): document Azure Search managed identity roles
Conclusion: failure
Current runner version: '2.335.1'
##[group]Runner Image Provisioner
Hosted Compute Agent
Version: 20260624.560
Commit: 925d229a51159bc391ae97e54a2dd1fe20af789d
Build Date:
Worker ID: {0d9b6db0-89fd-4c3b-a3c0-cd2e35295af9}
Azure Region: northcentralus
##[endgroup]
##[group]Operating System
Ubuntu
24.04.4
LTS
##[endgroup]
##[group]Runner Image
Image: ubuntu-24.04
Version: 20260628.225.1
Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20260628.225/images/ubuntu/Ubuntu2404-Readme.md
Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20260628.225
##[endgroup]
##[group]GITHUB_TOKEN Permissions
Contents: read
Metadata: read
##[endgroup]
Secret source: Actions
Prepare workflow directory
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v4' (SHA:34e114876b0b11c390a56381ad16ebd13914f8d5)
Download action repository 'actions/setup-python@v5' (SHA:a26af69be951a213d495a4c3e4e4022e16d87065)
Download action repository 'astral-sh/setup-uv@v4' (SHA:38f3f104447c67c051c4a08e39b64a148898af3a)
Complete job name: Script Validation
Node 20 is being deprecated. This workflow is running with Node 24 by default. If you need to temporarily use Node 20, you can set the ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true environment variable. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
##[group]Run actions/checkout@v4
with:
repository: NVIDIA-AI-Blueprints/aiq
***REDACTED***
ssh-strict: true
ssh-user: git
persist-credentials: true
clean: true
sparse-checkout-cone-mode: true
fetch-depth: 1
fetch-tags: false
show-progress: true
lfs: false
submodules: false
set-safe-directory: true
##[endgroup]
Syncing repository: NVIDIA-AI-Blueprints/aiq
##[group]Getting Git version info
Working directory is '/home/runner/work/aiq/aiq'
[command]/usr/bin/git versio...
🧰 Additional context used
📓 Path-based instructions (7)
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/examples/index.mddocs/source/index.mddocs/source/customization/knowledge-layer.mddocs/source/customization/configuration-reference.mddocs/source/examples/azure-ai-search.md
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
docs/source/examples/index.mdsources/knowledge_layer/src/azure_ai_search/__init__.pydocs/source/index.mdsources/knowledge_layer/pyproject.tomlsources/knowledge_layer/src/azure_ai_search/README.mdsources/knowledge_layer/src/__init__.pysources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mddocs/source/customization/knowledge-layer.mdtests/knowledge_layer_tests/run_adapter_compliance.pydocs/source/customization/configuration-reference.mdsources/knowledge_layer/README.mdsources/knowledge_layer/src/register.pytests/knowledge_layer_tests/test_azure_ai_search.pydocs/source/examples/azure-ai-search.mdsources/knowledge_layer/src/azure_ai_search/adapter.py
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/examples/index.mddocs/source/index.mddocs/source/customization/knowledge-layer.mddocs/source/customization/configuration-reference.mddocs/source/examples/azure-ai-search.md
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
sources/knowledge_layer/src/azure_ai_search/__init__.pysources/knowledge_layer/src/__init__.pytests/knowledge_layer_tests/run_adapter_compliance.pysources/knowledge_layer/src/register.pytests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/azure_ai_search/adapter.py
sources/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
New tools and data sources must be NAT functions registered with
@register_functiondecorator
Files:
sources/knowledge_layer/src/azure_ai_search/__init__.pysources/knowledge_layer/src/__init__.pysources/knowledge_layer/src/register.pysources/knowledge_layer/src/azure_ai_search/adapter.py
{src/aiq_agent/knowledge/**,sources/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.
Files:
sources/knowledge_layer/src/azure_ai_search/__init__.pysources/knowledge_layer/pyproject.tomlsources/knowledge_layer/src/azure_ai_search/README.mdsources/knowledge_layer/src/__init__.pysources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mdsources/knowledge_layer/README.mdsources/knowledge_layer/src/register.pysources/knowledge_layer/src/azure_ai_search/adapter.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/knowledge_layer_tests/test_azure_ai_search.py
🪛 ast-grep (0.44.1)
tests/knowledge_layer_tests/test_azure_ai_search.py
[warning] 52-52: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.search(rf"{field} {operator} '((?:''|[^'])*)'", filter_text)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
sources/knowledge_layer/src/azure_ai_search/adapter.py
[info] 175-175: use jsonify instead of json.dumps for JSON output
Context: json.dumps(marker, separators=(",", ":"), sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 250-250: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload, default=_json_default, ensure_ascii=False, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 770-770: use jsonify instead of json.dumps for JSON output
Context: json.dumps(metadata, separators=(",", ":"), sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 GitHub Actions: AIQ CI / 0_Lint and Hooks.txt
tests/knowledge_layer_tests/test_azure_ai_search.py
[error] 283-283: detect-secrets failed (exit code 3): Potential secret detected (Secret Keyword) at tests/knowledge_layer_tests/test_azure_ai_search.py:283.
[error] 284-284: detect-secrets failed (exit code 3): Potential secret detected (Secret Keyword) at tests/knowledge_layer_tests/test_azure_ai_search.py:284.
🪛 GitHub Actions: AIQ CI / Lint and Hooks
tests/knowledge_layer_tests/test_azure_ai_search.py
[error] 283-283: pre-commit hook 'detect-secrets' failed (exit code 3). Potential secret detected (Secret Type: Secret Keyword) at tests/knowledge_layer_tests/test_azure_ai_search.py:283.
[error] 284-284: pre-commit hook 'detect-secrets' failed (exit code 3). Potential secret detected (Secret Type: Secret Keyword) at tests/knowledge_layer_tests/test_azure_ai_search.py:284.
🪛 markdownlint-cli2 (0.22.1)
docs/source/customization/knowledge-layer.md
[warning] 160-160: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🔇 Additional comments (24)
tests/knowledge_layer_tests/test_azure_ai_search.py (2)
50-54: Skip: ReDoS hint is a false positive in this test helper.
_literal'sfield/operatorargs are only ever called with hardcoded literals ("file_id","file_name","id","gt") from within this test file, not attacker-controlled input, so the flagged pattern-injection risk doesn't apply here.
1-49: LGTM!Also applies to: 55-275, 296-765
tests/knowledge_layer_tests/run_adapter_compliance.py (1)
21-22: LGTM!Also applies to: 97-97, 393-405, 458-463
sources/knowledge_layer/src/azure_ai_search/adapter.py (5)
239-264: LGTM!
814-845: LGTM!
80-115: LGTM!
847-874: LGTM!
977-982: 🩺 Stability & AvailabilityConfirm the delete/read-after-delete contract here.
delete_collection()only returnsTrueif an immediateget_index()raisesResourceNotFoundError; if Azure Search can briefly expose a stale index afterdelete_index(), this will report a false failure. Either treatdelete_index()as authoritative or replace the reread with bounded polling.sources/knowledge_layer/src/register.py (3)
100-104: LGTM!Also applies to: 109-135, 136-172
218-241:azure_ai_searchbranch and unknown-backend message look correct.Consistent with the
BackendTypeliteral and existing branch style forllamaindex/foundational_rag.
92-99: 🎯 Functional CorrectnessRecheck
azure_search_auth_modeinference for explicit keys. Ifazure_search_api_keycan be provided withoutazure_search_auth_mode, the model still defaults tomanaged_identityand can skip the key path; handle that in the model validator or make the default depend on the supplied key.sources/knowledge_layer/pyproject.toml (2)
21-26: LGTM!
60-68: 📐 Maintainability & Code QualityAlign the shared dependency ranges across extras
Keepllama-index-core,docx2txt, andpypdfon the same constraints inazure_ai_searchandllamaindex;allpulls both extras together, so mismatched pins can make resolution fragile.sources/knowledge_layer/src/__init__.py (1)
25-25: LGTM!sources/knowledge_layer/src/azure_ai_search/__init__.py (1)
1-9: LGTM!sources/knowledge_layer/README.md (1)
15-33: LGTM!sources/knowledge_layer/src/azure_ai_search/README.md (2)
22-27: 📐 Maintainability & Code Quality | 💤 Low valueDoc's auth guidance doesn't mention the auth-mode/api-key coupling gap flagged in
register.py.Once the
register.pydefault-inference fix (see companion comment) lands, worth a one-line note here thatazure_search_auth_mode: api_keyshould be set explicitly when supplyingAZURE_SEARCH_API_KEYoutside the documented env-var flow.
1-92: LGTM!docs/source/customization/configuration-reference.md (1)
220-221: LGTM!Also applies to: 251-268
docs/source/customization/knowledge-layer.md (1)
46-46: LGTM!Also applies to: 56-57, 76-76, 250-256, 437-438
docs/source/examples/azure-ai-search.md (1)
1-82: LGTM!docs/source/examples/index.md (1)
15-15: LGTM!docs/source/index.md (1)
142-142: LGTM!sources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.md (1)
38-48: LGTM!Also applies to: 65-65, 858-863
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
@harmke everything worked great E2E, could you please address the remaining comments would love to get this into the 2.2 release! |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/source/customization/knowledge-layer.md`:
- Around line 179-183: Correct the migration wording in the Azure
knowledge-layer documentation and the duplicate Azure AI Search example
documentation: do not state that schema-version-1 indexes are ignored; describe
only unsupported or legacy schema versions as requiring re-ingestion, while
preserving that the current adapter uses schema version 1 and generates
knowledge-v1 indexes.
In `@sources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.md`:
- Around line 1047-1052: Add AZURE_CLIENT_ID to the environment-variable
reference table near the other Azure configuration variables, describing it as
the client ID for the user-assigned managed identity used by
DefaultAzureCredential.
In `@sources/knowledge_layer/src/azure_ai_search/adapter.py`:
- Around line 620-629: Move the synchronous collection lifecycle operations out
of async event-loop handlers: update the collection route handlers to invoke
ingestor.create_collection() and ingestor.delete_collection() via
asyncio.to_thread or an executor, or convert those handlers to synchronous
functions. Ensure all affected create/delete paths in the collections routes no
longer directly call these methods, which may block in _wait_for_search_state().
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: b44dca8a-e418-4bac-b746-1f552dfac181
📒 Files selected for processing (11)
.secrets.baselinedocs/source/customization/configuration-reference.mddocs/source/customization/knowledge-layer.mddocs/source/examples/azure-ai-search.mddocs/source/examples/index.mdsources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mdsources/knowledge_layer/README.mdsources/knowledge_layer/src/azure_ai_search/README.mdsources/knowledge_layer/src/azure_ai_search/adapter.pysources/knowledge_layer/src/register.pytests/knowledge_layer_tests/test_azure_ai_search.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/examples/index.mddocs/source/customization/configuration-reference.mddocs/source/examples/azure-ai-search.mddocs/source/customization/knowledge-layer.md
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
docs/source/examples/index.mdsources/knowledge_layer/README.mddocs/source/customization/configuration-reference.mdsources/knowledge_layer/src/azure_ai_search/README.mddocs/source/examples/azure-ai-search.mdsources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mddocs/source/customization/knowledge-layer.mdtests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/register.pysources/knowledge_layer/src/azure_ai_search/adapter.py
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/examples/index.mddocs/source/customization/configuration-reference.mddocs/source/examples/azure-ai-search.mddocs/source/customization/knowledge-layer.md
{src/aiq_agent/knowledge/**,sources/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.
Files:
sources/knowledge_layer/README.mdsources/knowledge_layer/src/azure_ai_search/README.mdsources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mdsources/knowledge_layer/src/register.pysources/knowledge_layer/src/azure_ai_search/adapter.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/register.pysources/knowledge_layer/src/azure_ai_search/adapter.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/knowledge_layer_tests/test_azure_ai_search.py
sources/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
New tools and data sources must be NAT functions registered with
@register_functiondecorator
Files:
sources/knowledge_layer/src/register.pysources/knowledge_layer/src/azure_ai_search/adapter.py
🪛 ast-grep (0.44.1)
sources/knowledge_layer/src/azure_ai_search/adapter.py
[info] 651-651: use jsonify instead of json.dumps for JSON output
Context: json.dumps(merged_metadata, separators=(",", ":"), sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 692-692: use jsonify instead of json.dumps for JSON output
Context: json.dumps(metadata, separators=(",", ":"), sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (22)
.secrets.baseline (1)
244-244: LGTM!Also applies to: 293-293
docs/source/customization/configuration-reference.md (1)
220-221: LGTM!Also applies to: 251-282
docs/source/customization/knowledge-layer.md (1)
46-46: LGTM!Also applies to: 56-57, 76-76, 159-177, 185-189, 250-256, 437-438
docs/source/examples/azure-ai-search.md (1)
1-85: LGTM!docs/source/examples/index.md (1)
15-15: LGTM!sources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.md (1)
12-12: LGTM!Also applies to: 39-49, 71-71, 191-191
sources/knowledge_layer/README.md (1)
18-20: LGTM!Also applies to: 30-30, 36-37
sources/knowledge_layer/src/azure_ai_search/README.md (1)
1-81: LGTM!tests/knowledge_layer_tests/test_azure_ai_search.py (11)
24-24: LGTM!Also applies to: 51-59, 67-157
241-274: LGTM!
295-307: LGTM!Also applies to: 332-342
404-432: LGTM!Also applies to: 446-484
496-506: LGTM!Also applies to: 542-561
575-598: LGTM!Also applies to: 624-654, 700-710
713-753: LGTM!Also applies to: 756-785, 788-826
828-890: LGTM!Also applies to: 905-940
943-976: LGTM!
992-1008: LGTM!
391-401: 🔒 Security & PrivacyNo issue:
azure_search_api_keyuses NAT’sOptionalSecretStr, so JSON dumps emit the underlying string for config reload rather than a maskedSecretStr.> Likely an incorrect or invalid review comment.sources/knowledge_layer/src/azure_ai_search/adapter.py (2)
518-525:filtersother than$filterare silently dropped.Raw
$filteris rejected, but a structured filter such as{"file_name": "report.pdf"}is neither applied nor rejected — the query silently runs unfiltered, unlike OpenSearch/Foundational RAG which honor the shared metadata-filter contract. Either apply supported keys or reject any non-empty unsupportedfilters.
85-103: LGTM!Also applies to: 141-150, 347-403, 447-500, 751-820, 904-959, 961-1033, 1121-1177, 1205-1267
sources/knowledge_layer/src/register.py (1)
37-37: LGTM!Also applies to: 259-283, 332-336, 531-531
|
/ok to test e74b76e |
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/knowledge_layer_tests/run_adapter_compliance.py (1)
405-407: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winVerify deletion by
file_id, not filename.The adapter permits same-name files with distinct IDs. The name-based check at Lines 413-417 reports a successful deletion as failed when another generation of that filename remains.
Proposed fix
- remaining_names = [f.file_name for f in remaining] + remaining_ids = {f.file_id for f in remaining} - if filename in remaining_names: - return False, f"File '{filename}' still exists after deletion" + if file_id in remaining_ids: + return False, f"File '{filename}' still exists after deletion"🤖 Prompt for 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. In `@tests/knowledge_layer_tests/run_adapter_compliance.py` around lines 405 - 407, Update the deletion verification in the test flow around self.ingestor.delete_file to locate the deleted file by file_id rather than filename. Keep same-name files with different IDs valid, and assert that the specific file_id is absent after deletion while preserving the existing success/failure behavior.
🤖 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 `@configs/config_web_azure_ai_search.yml`:
- Around line 1-4: Update the feature summary in the Web mode configuration
comments to describe Paper search as optional rather than enabled by default,
matching the commented-out paper search source and paper_search_tool entries.
In `@tests/knowledge_layer_tests/run_adapter_compliance.py`:
- Around line 30-34: Update the adapter compliance example command in
run_adapter_compliance.py documentation to invoke the script through uv run
instead of bare python, while preserving the existing backend and config
arguments.
---
Outside diff comments:
In `@tests/knowledge_layer_tests/run_adapter_compliance.py`:
- Around line 405-407: Update the deletion verification in the test flow around
self.ingestor.delete_file to locate the deleted file by file_id rather than
filename. Keep same-name files with different IDs valid, and assert that the
specific file_id is absent after deletion while preserving the existing
success/failure behavior.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 7ae9ed76-8e45-45f4-86cd-507203814e55
📒 Files selected for processing (10)
configs/config_web_azure_ai_search.ymldeploy/.env.exampledocs/source/customization/configuration-reference.mddocs/source/customization/knowledge-layer.mddocs/source/examples/azure-ai-search.mddocs/source/examples/index.mdsources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mdsources/knowledge_layer/src/azure_ai_search/adapter.pytests/knowledge_layer_tests/run_adapter_compliance.pytests/knowledge_layer_tests/test_azure_ai_search.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/examples/index.mddocs/source/customization/knowledge-layer.mddocs/source/examples/azure-ai-search.mddocs/source/customization/configuration-reference.md
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
docs/source/examples/index.mdconfigs/config_web_azure_ai_search.ymldocs/source/customization/knowledge-layer.mddocs/source/examples/azure-ai-search.mdtests/knowledge_layer_tests/run_adapter_compliance.pydocs/source/customization/configuration-reference.mdsources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mdtests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/azure_ai_search/adapter.py
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/examples/index.mddocs/source/customization/knowledge-layer.mddocs/source/examples/azure-ai-search.mddocs/source/customization/configuration-reference.md
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
configs/config_web_azure_ai_search.yml
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/knowledge_layer_tests/run_adapter_compliance.pytests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/azure_ai_search/adapter.py
{src/aiq_agent/knowledge/**,sources/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.
Files:
sources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.mdsources/knowledge_layer/src/azure_ai_search/adapter.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/knowledge_layer_tests/test_azure_ai_search.py
sources/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
New tools and data sources must be NAT functions registered with
@register_functiondecorator
Files:
sources/knowledge_layer/src/azure_ai_search/adapter.py
🔇 Additional comments (11)
deploy/.env.example (1)
169-178: LGTM!tests/knowledge_layer_tests/run_adapter_compliance.py (1)
109-109: LGTM!Also applies to: 474-474
sources/knowledge_layer/src/azure_ai_search/adapter.py (2)
518-524: LGTM!
1233-1237: LGTM!tests/knowledge_layer_tests/test_azure_ai_search.py (1)
417-422: 📐 Maintainability & Code QualityRun the Python validation for the touched files.
uv run ruff check,uv run ruff format --check, anduv run pytest tests/knowledge_layer_testsforsources/knowledge_layer/src/azure_ai_search/adapter.py,tests/knowledge_layer_tests/run_adapter_compliance.py, andtests/knowledge_layer_tests/test_azure_ai_search.py.docs/source/customization/configuration-reference.md (1)
220-282: LGTM!Also applies to: 539-539
docs/source/customization/knowledge-layer.md (1)
46-57: LGTM!Also applies to: 76-76, 159-188, 249-255, 436-437
docs/source/examples/azure-ai-search.md (1)
27-44: LGTM!Also applies to: 59-88
docs/source/examples/index.md (1)
15-15: LGTM!sources/knowledge_layer/KNOWLEDGE-LAYER-SETUP.md (1)
12-12: LGTM!Also applies to: 39-49, 71-71, 191-191, 1049-1053
configs/config_web_azure_ai_search.yml (1)
6-211: LGTM!
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@sources/knowledge_layer/src/azure_ai_search/adapter.py`:
- Around line 845-873: Track whether the setup flow created the collection
manifest in the block around _get_collection_manifest and
_write_collection_manifest. In the exception rollback path, when that flag is
set, delete the corresponding collection manifest along with the existing
_RECORD_FILE cleanup, while preserving rollback error reporting and behavior for
pre-existing collections.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 191d085b-149d-4912-b384-d73a0b798bb4
📒 Files selected for processing (2)
sources/knowledge_layer/src/azure_ai_search/adapter.pytests/knowledge_layer_tests/test_azure_ai_search.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/azure_ai_search/adapter.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/knowledge_layer_tests/test_azure_ai_search.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
tests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/azure_ai_search/adapter.py
sources/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
New tools and data sources must be NAT functions registered with
@register_functiondecorator
Files:
sources/knowledge_layer/src/azure_ai_search/adapter.py
{src/aiq_agent/knowledge/**,sources/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.
Files:
sources/knowledge_layer/src/azure_ai_search/adapter.py
🔇 Additional comments (4)
sources/knowledge_layer/src/azure_ai_search/adapter.py (3)
780-805: LGTM!
806-819: LGTM!
1079-1102: LGTM!tests/knowledge_layer_tests/test_azure_ai_search.py (1)
598-602: LGTM!Also applies to: 605-652, 655-676
Signed-off-by: Harmke Alkemade <halkemade@nvidia.com>
|
/ok to test 4f7fdc7 |
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
/ok to test 5a00cf6 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sources/knowledge_layer/src/azure_ai_search/adapter.py (3)
759-762: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUnhandled metadata-serialization error breaks
submit_job's never-raise contract.
json.dumps(file_metadata, ...)here validates JSON-serializability but discards the result and isn't wrapped in atry/except. Every other failure path insubmit_job(invalid paths, thread-start failure) gracefully records aFAILEDjob and returnsjob_id; a non-serializablemetadatadict instead raisesTypeErrorstraight out ofsubmit_job, which can crash the caller (e.g.upload_file, an HTTP route) instead of surfacing a normal failed-job result.🐛 Proposed fix
- file_metadata = job_config.get("metadata") or {} - json.dumps(file_metadata, separators=(",", ":"), sort_keys=True) + file_metadata = job_config.get("metadata") or {} + try: + json.dumps(file_metadata, separators=(",", ":"), sort_keys=True) + except TypeError as error: + with self._jobs_lock: + self._jobs[job_id] = IngestionJobStatus( + job_id=job_id, + status=JobState.FAILED, + collection_name=collection_name, + backend=_BACKEND_NAME, + submitted_at=_utc_now(), + completed_at=_utc_now(), + total_files=len(file_paths), + error_message=f"Metadata is not JSON-serializable: {error}", + ) + return job_id🤖 Prompt for 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. In `@sources/knowledge_layer/src/azure_ai_search/adapter.py` around lines 759 - 762, Update submit_job around the file_metadata JSON validation to catch serialization failures such as TypeError, record the job as FAILED using the existing failure-handling path, and return job_id without raising. Preserve the current compact/sorted serialization validation for valid metadata and the existing behavior for other submit_job failures.
1240-1249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPossible
IndexErrorif the file disappears betweenupload_file's check andsubmit_job's validation.
upload_filecheckspath.is_file()at Line 1232, butsubmit_jobre-checksPath(path).is_file()(Line 760) and skips paths that fail. If the file is removed in that narrow window,submit_jobreturns a job withtotal_files=1butfile_details=[](the "No valid file paths provided" branch), andself._jobs[job_id].file_details[0]here raisesIndexErrorinstead of returning a gracefulFailStatus.FAILEDFileInfolike the pre-check at Line 1232-1239 does.🛡️ Proposed guard
with self._jobs_lock: - file_id = self._jobs[job_id].file_details[0].file_id + job = self._jobs[job_id] + if not job.file_details: + return FileInfo( + file_id=str(uuid.uuid4()), + file_name=path.name, + collection_name=collection_name, + status=FileStatus.FAILED, + error_message=job.error_message or f"File not found: {file_path}", + ) + file_id = job.file_details[0].file_id info = self._files[file_id].model_copy(deep=True)🤖 Prompt for 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. In `@sources/knowledge_layer/src/azure_ai_search/adapter.py` around lines 1240 - 1249, Guard the file_details access in upload_file after submit_job, before indexing file_details[0]. If the submitted job has no file details, return the same graceful FailStatus.FAILED FileInfo used by the earlier path validation instead of raising IndexError; retain the existing metadata and job_id population for valid submissions.
1057-1068: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout around the summary LLM call.
self._summary_llm.invoke(prompt)runs synchronously inside the per-file ingestion path with no timeout. A hung LLM call blocks the ingestion worker thread indefinitely, leaving the file stuck atINGESTINGwith no way to recover short of restarting the process.🤖 Prompt for 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. In `@sources/knowledge_layer/src/azure_ai_search/adapter.py` around lines 1057 - 1068, The synchronous call in _generate_summary must have a bounded timeout so a hung summary request cannot block ingestion indefinitely. Configure or pass the supported timeout through self._summary_llm.invoke(prompt), preserving the existing exception logging and None fallback when the call exceeds the limit or otherwise fails.
🤖 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 `@docs/source/customization/configuration-reference.md`:
- Around line 597-605: Update the top-level workflow configuration count in the
introduction of the configuration reference to match the ten profiles listed in
the table, or remove the hard-coded count while preserving the repository paths
and examples.
- Around line 305-308: Keep Azure embedding configuration consistent across all
documented tables: in docs/source/customization/configuration-reference.md lines
305-308, mark embed_model and embed_base_url as Azure-supported alongside
embed_dim; in lines 322-323, extend the embedding model and endpoint rows to
include Azure; and in docs/source/customization/knowledge-layer.md line 499, add
Azure to the embedding environment-variable scope and document AIQ_EMBED_DIM.
In `@docs/source/customization/knowledge-layer.md`:
- Around line 57-58: Update the azure_ai_search feature summary in the
knowledge-layer documentation to describe hybrid retrieval only, removing the
unsupported vector and semantic-ranked retrieval claims while preserving the
existing embedding and namespaced-index details.
---
Outside diff comments:
In `@sources/knowledge_layer/src/azure_ai_search/adapter.py`:
- Around line 759-762: Update submit_job around the file_metadata JSON
validation to catch serialization failures such as TypeError, record the job as
FAILED using the existing failure-handling path, and return job_id without
raising. Preserve the current compact/sorted serialization validation for valid
metadata and the existing behavior for other submit_job failures.
- Around line 1240-1249: Guard the file_details access in upload_file after
submit_job, before indexing file_details[0]. If the submitted job has no file
details, return the same graceful FailStatus.FAILED FileInfo used by the earlier
path validation instead of raising IndexError; retain the existing metadata and
job_id population for valid submissions.
- Around line 1057-1068: The synchronous call in _generate_summary must have a
bounded timeout so a hung summary request cannot block ingestion indefinitely.
Configure or pass the supported timeout through
self._summary_llm.invoke(prompt), preserving the existing exception logging and
None fallback when the call exceeds the limit or otherwise fails.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: ef20def1-d9fe-41c1-90cb-278e60aa3f69
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.secrets.baselineconfigs/config_web_azure_ai_search.ymldeploy/.env.exampledocs/source/customization/configuration-reference.mddocs/source/customization/knowledge-layer.mddocs/source/examples/index.mddocs/source/index.mdsources/knowledge_layer/src/azure_ai_search/adapter.pytests/knowledge_layer_tests/test_azure_ai_search.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/examples/index.mddocs/source/index.mddocs/source/customization/configuration-reference.mddocs/source/customization/knowledge-layer.md
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
docs/source/examples/index.mddocs/source/index.mdconfigs/config_web_azure_ai_search.ymldocs/source/customization/configuration-reference.mddocs/source/customization/knowledge-layer.mdtests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/azure_ai_search/adapter.py
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/examples/index.mddocs/source/index.mddocs/source/customization/configuration-reference.mddocs/source/customization/knowledge-layer.md
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
configs/config_web_azure_ai_search.yml
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/knowledge_layer_tests/test_azure_ai_search.pysources/knowledge_layer/src/azure_ai_search/adapter.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/knowledge_layer_tests/test_azure_ai_search.py
sources/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
New tools and data sources must be NAT functions registered with
@register_functiondecorator
Files:
sources/knowledge_layer/src/azure_ai_search/adapter.py
{src/aiq_agent/knowledge/**,sources/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/knowledge/**,sources/**}: Review data-source and knowledge-layer changes for optional dependency boundaries, external API error handling,
retry/rate-limit behavior, deterministic tests, and registration consistency. New source packages should include
package metadata, plugin registration when applicable, and source-level tests.
Files:
sources/knowledge_layer/src/azure_ai_search/adapter.py
🪛 Betterleaks (1.6.1)
.secrets.baseline
[high] 206-206: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 213-213: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 220-220: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 227-227: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 234-234: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 243-243: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 250-250: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 257-257: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 264-264: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 271-271: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 280-280: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
docs/source/customization/configuration-reference.md
[style] ~442-~442: Consider using “incompatible” to avoid wordiness.
Context: ...only when the active source formats are not compatible with verification. | | skills | objec...
(NOT_ABLE_PREMIUM)
🔇 Additional comments (11)
docs/source/customization/configuration-reference.md (1)
82-82: LGTM!Also applies to: 155-158, 207-232, 259-259, 275-290, 294-294, 309-321, 325-326, 371-371, 403-405, 416-427, 438-455, 487-491, 606-612
docs/source/customization/knowledge-layer.md (2)
46-46: LGTM!Also applies to: 81-81, 171-190, 195-200, 301-307, 488-489
18-18: 🎯 Functional CorrectnessClarify Azure support in the shared capability docs. If Azure supports UI-created
s_<uuid>collections, add it to the backend-agnostic and session-collection lists; otherwise state that limitation explicitly.docs/source/examples/index.md (1)
15-15: LGTM!docs/source/index.md (1)
16-16: LGTM!Also applies to: 102-102, 145-145
configs/config_web_azure_ai_search.yml (1)
4-4: LGTM!.secrets.baseline (1)
136-136: LGTM!Also applies to: 145-145, 172-172, 202-274, 276-283, 300-300, 309-309, 354-354, 358-358
sources/knowledge_layer/src/azure_ai_search/adapter.py (2)
845-873: Auto-created collection manifest is still not rolled back on setup failure.This is the same code flagged in a prior review: when
collection is Noneat Line 850,_write_collection_manifest(collection_name)creates a newactivemanifest, but theexceptblock's rollback (Lines 858-869) only deletes_RECORD_FILEids fromjob.file_details— it never deletes the collection manifest it just created. A later failure (e.g. the first_write_file_manifestcall) leaves an empty, orphanedactivecollection visible vialist_collections()/get_collection().Per the prior thread, this was acknowledged and deferred rather than fixed; flagging again since the current diff still contains the exact gap and no tracking issue evidence is present in this context.
1-83: LGTM!Also applies to: 85-234, 237-403, 406-469, 591-630, 631-750, 821-834, 926-1055, 1070-1151, 1153-1224, 1251-1325
deploy/.env.example (1)
173-182: LGTM!tests/knowledge_layer_tests/test_azure_ai_search.py (1)
598-602: LGTM!Also applies to: 605-652, 655-676
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sources/knowledge_layer/src/azure_ai_search/adapter.py (3)
759-762: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUnhandled metadata-serialization error breaks
submit_job's never-raise contract.
json.dumps(file_metadata, ...)here validates JSON-serializability but discards the result and isn't wrapped in atry/except. Every other failure path insubmit_job(invalid paths, thread-start failure) gracefully records aFAILEDjob and returnsjob_id; a non-serializablemetadatadict instead raisesTypeErrorstraight out ofsubmit_job, which can crash the caller (e.g.upload_file, an HTTP route) instead of surfacing a normal failed-job result.🐛 Proposed fix
- file_metadata = job_config.get("metadata") or {} - json.dumps(file_metadata, separators=(",", ":"), sort_keys=True) + file_metadata = job_config.get("metadata") or {} + try: + json.dumps(file_metadata, separators=(",", ":"), sort_keys=True) + except TypeError as error: + with self._jobs_lock: + self._jobs[job_id] = IngestionJobStatus( + job_id=job_id, + status=JobState.FAILED, + collection_name=collection_name, + backend=_BACKEND_NAME, + submitted_at=_utc_now(), + completed_at=_utc_now(), + total_files=len(file_paths), + error_message=f"Metadata is not JSON-serializable: {error}", + ) + return job_id🤖 Prompt for 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. In `@sources/knowledge_layer/src/azure_ai_search/adapter.py` around lines 759 - 762, Update submit_job around the file_metadata JSON validation to catch serialization failures such as TypeError, record the job as FAILED using the existing failure-handling path, and return job_id without raising. Preserve the current compact/sorted serialization validation for valid metadata and the existing behavior for other submit_job failures.
1240-1249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPossible
IndexErrorif the file disappears betweenupload_file's check andsubmit_job's validation.
upload_filecheckspath.is_file()at Line 1232, butsubmit_jobre-checksPath(path).is_file()(Line 760) and skips paths that fail. If the file is removed in that narrow window,submit_jobreturns a job withtotal_files=1butfile_details=[](the "No valid file paths provided" branch), andself._jobs[job_id].file_details[0]here raisesIndexErrorinstead of returning a gracefulFailStatus.FAILEDFileInfolike the pre-check at Line 1232-1239 does.🛡️ Proposed guard
with self._jobs_lock: - file_id = self._jobs[job_id].file_details[0].file_id + job = self._jobs[job_id] + if not job.file_details: + return FileInfo( + file_id=str(uuid.uuid4()), + file_name=path.name, + collection_name=collection_name, + status=FileStatus.FAILED, + error_message=job.error_message or f"File not found: {file_path}", + ) + file_id = job.file_details[0].file_id info = self._files[file_id].model_copy(deep=True)🤖 Prompt for 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. In `@sources/knowledge_layer/src/azure_ai_search/adapter.py` around lines 1240 - 1249, Guard the file_details access in upload_file after submit_job, before indexing file_details[0]. If the submitted job has no file details, return the same graceful FailStatus.FAILED FileInfo used by the earlier path validation instead of raising IndexError; retain the existing metadata and job_id population for valid submissions.
1057-1068: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout around the summary LLM call.
self._summary_llm.invoke(prompt)runs synchronously inside the per-file ingestion path with no timeout. A hung LLM call blocks the ingestion worker thread indefinitely, leaving the file stuck atINGESTINGwith no way to recover short of restarting the process.🤖 Prompt for 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. In `@sources/knowledge_layer/src/azure_ai_search/adapter.py` around lines 1057 - 1068, The synchronous call in _generate_summary must have a bounded timeout so a hung summary request cannot block ingestion indefinitely. Configure or pass the supported timeout through self._summary_llm.invoke(prompt), preserving the existing exception logging and None fallback when the call exceeds the limit or otherwise fails.
🤖 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 `@docs/source/customization/configuration-reference.md`:
- Around line 597-605: Update the top-level workflow configuration count in the
introduction of the configuration reference to match the ten profiles listed in
the table, or remove the hard-coded count while preserving the repository paths
and examples.
- Around line 305-308: Keep Azure embedding configuration consistent across all
documented tables: in docs/source/customization/configuration-reference.md lines
305-308, mark embed_model and embed_base_url as Azure-supported alongside
embed_dim; in lines 322-323, extend the embedding model and endpoint rows to
include Azure; and in docs/source/customization/knowledge-layer.md line 499, add
Azure to the embedding environment-variable scope and document AIQ_EMBED_DIM.
In `@docs/source/customization/knowledge-layer.md`:
- Around line 57-58: Update the azure_ai_search feature summary in the
knowledge-layer documentation to describe hybrid retrieval only, removing the
unsupported vector and semantic-ranked retrieval claims while preserving the
existing embedding and namespaced-index details.
---
Outside diff comments:
In `@sources/knowledge_layer/src/azure_ai_search/adapter.py`:
- Around line 759-762: Update submit_job around the file_metadata JSON
validation to catch serialization failures such as TypeError, record the job as
FAILED using the existing failure-handling path, and return job_id without
raising. Preserve the current compact/sorted serialization validation for valid
metadata and the existing behavior for other submit_job failures.
- Around line 1240-1249: Guard the file_details access in upload_file after
submit_job, before indexing file_details[0]. If the submitted job has no file
details, return the same graceful FailStatus.FAILED FileInfo used by the earlier
path validation instead of raising IndexError; retain the existing metadata and
job_id population for valid submissions.
- Around line 1057-1068: The synchronous call in _generate_summary must have a
bounded timeout so a hung summary request cannot block ingestion indefinitely.
Configure or pass the supported timeout through
self._summary_llm.invoke(prompt), preserving the existing exception logging and
None fallback when the call exceeds the limit or otherwise fails.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: ef20def1-d9fe-41c1-90cb-278e60aa3f69
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.secrets.baselineconfigs/config_web_azure_ai_search.ymldeploy/.env.exampledocs/source/customization/configuration-reference.mddocs/source/customization/knowledge-layer.mddocs/source/examples/index.mddocs/source/index.mdsources/knowledge_layer/src/azure_ai_search/adapter.pytests/knowledge_layer_tests/test_azure_ai_search.py
📜 Review details
🔇 Additional comments (11)
docs/source/customization/configuration-reference.md (1)
82-82: LGTM!Also applies to: 155-158, 207-232, 259-259, 275-290, 294-294, 309-321, 325-326, 371-371, 403-405, 416-427, 438-455, 487-491, 606-612
docs/source/customization/knowledge-layer.md (2)
46-46: LGTM!Also applies to: 81-81, 171-190, 195-200, 301-307, 488-489
18-18: 🎯 Functional CorrectnessClarify Azure support in the shared capability docs. If Azure supports UI-created
s_<uuid>collections, add it to the backend-agnostic and session-collection lists; otherwise state that limitation explicitly.docs/source/examples/index.md (1)
15-15: LGTM!docs/source/index.md (1)
16-16: LGTM!Also applies to: 102-102, 145-145
configs/config_web_azure_ai_search.yml (1)
4-4: LGTM!.secrets.baseline (1)
136-136: LGTM!Also applies to: 145-145, 172-172, 202-274, 276-283, 300-300, 309-309, 354-354, 358-358
sources/knowledge_layer/src/azure_ai_search/adapter.py (2)
845-873: Auto-created collection manifest is still not rolled back on setup failure.This is the same code flagged in a prior review: when
collection is Noneat Line 850,_write_collection_manifest(collection_name)creates a newactivemanifest, but theexceptblock's rollback (Lines 858-869) only deletes_RECORD_FILEids fromjob.file_details— it never deletes the collection manifest it just created. A later failure (e.g. the first_write_file_manifestcall) leaves an empty, orphanedactivecollection visible vialist_collections()/get_collection().Per the prior thread, this was acknowledged and deferred rather than fixed; flagging again since the current diff still contains the exact gap and no tracking issue evidence is present in this context.
1-83: LGTM!Also applies to: 85-234, 237-403, 406-469, 591-630, 631-750, 821-834, 926-1055, 1070-1151, 1153-1224, 1251-1325
deploy/.env.example (1)
173-182: LGTM!tests/knowledge_layer_tests/test_azure_ai_search.py (1)
598-602: LGTM!Also applies to: 605-652, 655-676
🛑 Comments failed to post (3)
docs/source/customization/configuration-reference.md (2)
305-308: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep Azure embedding configuration consistent across both documentation tables.
The runtime passes the embedding model, endpoint, and dimension into Azure, but the documentation scopes these settings to other backends or omits them.
docs/source/customization/configuration-reference.md#L305-L308: Markembed_modelandembed_base_urlas Azure-supported alongsideembed_dim.docs/source/customization/configuration-reference.md#L322-L323: Extend the backend scope of the embedding model and endpoint rows to include Azure.docs/source/customization/knowledge-layer.md#L499-L499: Add Azure to the embedding environment-variable scope and documentAIQ_EMBED_DIM.📍 Affects 2 files
docs/source/customization/configuration-reference.md#L305-L308(this comment)docs/source/customization/configuration-reference.md#L322-L323docs/source/customization/knowledge-layer.md#L499-L499🤖 Prompt for 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. In `@docs/source/customization/configuration-reference.md` around lines 305 - 308, Keep Azure embedding configuration consistent across all documented tables: in docs/source/customization/configuration-reference.md lines 305-308, mark embed_model and embed_base_url as Azure-supported alongside embed_dim; in lines 322-323, extend the embedding model and endpoint rows to include Azure; and in docs/source/customization/knowledge-layer.md line 499, add Azure to the embedding environment-variable scope and document AIQ_EMBED_DIM.Source: Path instructions
597-605: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the provided-config count.
The introduction says there are nine top-level workflow configurations, but the table now lists ten, including
config_web_azure_ai_search.yml. Update the count to 10 or avoid hard-coding it.As per path instructions, documentation must keep repository-layout references and examples accurate.
🤖 Prompt for 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. In `@docs/source/customization/configuration-reference.md` around lines 597 - 605, Update the top-level workflow configuration count in the introduction of the configuration reference to match the ten profiles listed in the table, or remove the hard-coded count while preserving the repository paths and examples.Source: Path instructions
docs/source/customization/knowledge-layer.md (1)
57-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail file='docs/source/customization/knowledge-layer.md' wc -l "$file" printf '\n--- lines 1-260 ---\n' sed -n '1,260p' "$file" | cat -n printf '\n--- search Azure retrieval wording ---\n' rg -n "azure_ai_search|vector, hybrid, semantic|always hybrid|hybrid retrieval|semantic-ranked" docs/source source configs -SRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 16860
🏁 Script executed:
set -euo pipefail printf '%s\n' '--- Azure example doc excerpt ---' sed -n '70,110p' docs/source/examples/azure-ai-search.md | cat -n printf '\n%s\n' '--- locate Azure implementation ---' fd -a "azure" sources/knowledge_layer/src printf '\n%s\n' '--- search mode/retrieval symbols in Azure source ---' rg -n "semantic|hybrid|vector|retriev|search mode|query_type|search_type|top_k" sources/knowledge_layer/src -SRepository: NVIDIA-AI-Blueprints/aiq
Length of output: 27369
Clarify the Azure retrieval mode.
docs/source/customization/knowledge-layer.md:57-58should not promise vector, hybrid, and semantic-ranked retrieval when the rest of the Azure docs describe the backend as hybrid-only. Keep the feature summary aligned with the adapter contract.🤖 Prompt for 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. In `@docs/source/customization/knowledge-layer.md` around lines 57 - 58, Update the azure_ai_search feature summary in the knowledge-layer documentation to describe hybrid retrieval only, removing the unsupported vector and semantic-ranked retrieval claims while preserving the existing embedding and namespaced-index details.Source: Path instructions
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
/ok to test 5a00cf6 |
3 participants
Overview
Adds Azure AI Search as a Knowledge Layer backend for AI-Q. This gives deployments a managed alternative to the existing LlamaIndex/ChromaDB and Foundational RAG backends while preserving the shared Knowledge API, NAT function registration, document summaries, and citation formatting.
The adapter generates embeddings client-side through the existing NVIDIA embedding integration and stores document chunks in one Azure AI Search index per logical collection.
This PR includes:
azure_ai_searchretriever and ingestor adapters with vector, hybrid, and opt-in semantic-ranked retrieval.AZURE_SEARCH_API_KEYor Azure identity authentication throughDefaultAzureCredential.Search Service ContributorandSearch Index Data Contributorroles.AIQ_EMBED_BASE_URLandAIQ_EMBED_MODEL, with Azure index dimensions configured throughAIQ_EMBED_DIM.This PR does not provision Azure infrastructure. The Azure AI Search service, required permissions, and embedding endpoint must already exist.
Validation
All commands were run from the repository root unless noted.
Adapter registration and factory compliance:
Documentation build:
The three Sphinx warnings are unresolved cross-reference warnings in unrelated existing pages (
architecture/agents/sandbox.mdandintegration/agent-skills.md).Live Azure service validation was also completed against a Basic-tier Azure AI Search service using NVIDIA-hosted
nvidia/nv-embed-v1embeddings (4096 dimensions). The full adapter compliance flow passed14/14under admin-key authentication, covering collection creation, TXT ingestion, asynchronous status polling, file listing, hybrid retrieval with semantic ranking, chunk normalization, file deletion, and collection cleanup.DefaultAzureCredentialalso selected a system-assigned managed identity in Azure Container Apps, and index creation/deletion, document upload/deletion, and querying all succeeded with the documentedSearch Service ContributorandSearch Index Data Contributorroles.git commit -sor an equivalent sign-off.Where should reviewers start?
Start with
sources/knowledge_layer/src/azure_ai_search/adapter.pyfor index ownership, schema validation, ingestion, replacement, and retrieval behavior. Then reviewsources/knowledge_layer/src/register.pyfor NAT configuration and authentication defaults, followed bytests/knowledge_layer_tests/test_azure_ai_search.pyfor the expected lifecycle and failure handling.The main design decision is that AI-Q creates and manages only namespaced indexes carrying its ownership marker; unmanaged or incompatible indexes are ignored rather than modified.
Related Issues
Summary by CodeRabbit