LCORE-2923: Add missing configuration fields to telemetry configuration snapshot [Opus Firefly test] - #2281
Draft
are-ces wants to merge 1 commit into
Draft
Conversation
Contributor
WalkthroughChangesExternal A2A delegation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Pydantic-AI agent
participant A2ADelegationCapability
participant A2AClientManager
participant Remote A2A agent
Pydantic-AI agent->>A2ADelegationCapability: delegate_to_agent(agent_name, task)
A2ADelegationCapability->>A2AClientManager: get_client(agent_name)
A2ADelegationCapability->>Remote A2A agent: send_message(task)
Remote A2A agent-->>A2ADelegationCapability: streamed response events
A2ADelegationCapability-->>Pydantic-AI agent: aggregated response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 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/devel_doc/openapi.json`:
- Around line 10930-10939: Bound the A2A agents collection at the source model
by adding an appropriate max-length constraint to the
A2AAgentsConfiguration.agents field, then regenerate docs/devel_doc/openapi.json
so the generated schema includes maxItems. Do not edit the generated OpenAPI
JSON directly.
In `@docs/user_doc/a2a_protocol.md`:
- Around line 316-317: Align the executor flow described around build_agent()
and run_stream_events() with the retained direct “Llama Stack Responses API”
flow, either by updating both diagrams to show the same execution path or by
explicitly documenting the adapter boundary between them. Ensure the final
documentation consistently explains how Pydantic-AI events map to the
A2A/Responses API events.
In `@examples/lightspeed-stack-a2a-agents.yaml`:
- Line 7: Update the documentation reference in the comment at the top of the
Lightspeed A2A agents configuration to point to docs/user_doc/a2a_protocol.md
instead of the stale docs/a2a_protocol.md path.
In `@src/a2a_client/capability.py`:
- Around line 88-91: Update the docstring for the affected capability helper to
rename the argument section header from “Args:” to “Parameters:”, matching the
convention used by sibling helpers in the same file; leave the parameter
descriptions unchanged.
- Around line 102-115: Broaden the exception handler around message creation and
_send_and_collect in the delegation flow to catch A2AClientError,
httpx.HTTPError, and OSError, matching the guard used by
A2AClientManager.initialize. Preserve the existing warning and graceful
degradation response, including the agent name and error details, for all
handled transport failures.
- Around line 167-183: Update the tuple-event handling around
_extract_text_from_artifact to import TaskStatusUpdateEvent from a2a.types and
use isinstance(update_event, TaskStatusUpdateEvent) for terminal-state
processing. Remove the redundant update_event is not None and hasattr status
checks while preserving the existing failure-state and message extraction
behavior.
In `@src/a2a_client/manager.py`:
- Around line 55-60: The A2A client must never attach bearer credentials to
cleartext URLs. In src/a2a_client/manager.py lines 55-60, validate configured
auth_token URLs as HTTPS and defensively skip or reject header injection before
the Authorization construction. In docs/user_doc/a2a_protocol.md lines 824-835,
change the token example to HTTPS and state that bearer authentication requires
TLS.
- Around line 103-110: Update A2AClientManager to retain each successfully
created injected httpx.AsyncClient from the connection flow, and close those
clients with await aclose() in A2AClientManager.close() alongside existing SDK
client cleanup; do not rely solely on Client.close(). Update
tests/unit/a2a_client/test_manager.py at lines 166-169 to verify injected
HTTP-client cleanup.
In `@src/configuration.py`:
- Around line 431-437: Update the `a2a_agents` property docstring to describe
the A2A agents configuration and add a Google-style `Raises` section documenting
`LogicError` when configuration is not loaded.
In `@src/models/config.py`:
- Around line 2036-2040: Update A2aaAAgentEndpointConfiguration.name with a
field_validator that rejects names whose stripped value is blank, while
preserving valid names. Add an after model_validator to A2AAgentsConfiguration
that detects duplicate agent names and raises a validation error before manager
initialization. Add regression tests covering whitespace-only names and
duplicate names.
In `@src/telemetry/configuration_snapshot.py`:
- Around line 482-486: Update the MaskingType.SENSITIVE branch in the
configuration serialization logic to return NOT_CONFIGURED for empty containers
as well as None and empty strings. Preserve CONFIGURED for non-empty values and
the existing _serialize_passthrough behavior for other masking types.
In `@tests/unit/a2a_client/test_capability.py`:
- Around line 106-127: Add a second async test alongside
test_returns_failure_string_on_a2a_client_error that makes
mock_client.send_message raise an httpx.ReadTimeout or OSError, invokes
delegate_to_agent through A2ADelegationCapability, and asserts it returns the
expected delegation-failure string rather than propagating the transport
exception.
- Around line 61-69: The existing test only verifies that list_agents is
registered and does not execute its closure. Add an async test for the
list_agents tool from A2ADelegationCapability.get_toolset, configure one agent
card with a None description, await the tool, and assert it maps agent names to
descriptions while returning an empty string for the missing description.
In `@tests/unit/a2a_client/test_manager.py`:
- Around line 15-17: Complete the docstrings for the affected test fixture and
test functions, including a Parameters: section for every injected fixture
argument. Use the actual parameter names and briefly describe each fixture’s
purpose, including _reset_singleton and the additional functions identified by
the review.
In `@tests/unit/telemetry/conftest.py`:
- Around line 672-685: Update the minimal configuration fixture’s
conversation_cache, a2a_state, and quota_handlers fields to use the same
default-factory-created instances as Configuration instead of None. Keep the
fixture otherwise unchanged so tests exercise the model’s real default shape.
In `@tests/unit/telemetry/test_configuration_snapshot_new_sections.py`:
- Around line 391-398: Strengthen the *_not_configured tests by removing
conditional guards and asserting the expected NOT_CONFIGURED value directly.
Update test_a2a_state_not_configured_when_none and the equivalent
conversation_cache, quota_handlers, splunk, and azure_entra_id tests to access
their sections unconditionally and require the documented exact value, following
the sibling module’s assertion pattern.
- Around line 1045-1052: Update
test_none_optional_sections_produce_not_configured to assert that each optional
section set to None produces the expected not_configured markers in the snapshot
output, rather than only checking JSON serialization. Reuse the existing
snapshot structure and marker symbols used by the surrounding tests, while
preserving the serialization assertion if still relevant.
- Around line 816-882: Define a shared NEW_SECTIONS constant for the 20 section
names and use it in both test_section_in_registry and the other parametrized
test instead of duplicating the list. Consolidate the duplicated registry-path
extraction into _registry_top_level_paths() and update
test_snapshot_only_contains_allowlisted_fields to reuse that helper in both
branches.
In `@tests/unit/telemetry/test_configuration_snapshot.py`:
- Around line 499-956: Remove the duplicated fully-populated assertions from the
existing snapshot test class, retaining only unique *_none and default-value
coverage that is absent from test_configuration_snapshot_new_sections.py. Reuse
that module’s full_snapshot and minimal_snapshot fixtures for retained tests
instead of rebuilding configurations with build_fully_populated_config() or
build_minimal_config() in each method, and remove the now-unnecessary
too-many-public-methods suppression if applicable.
🪄 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: Pro Plus
Run ID: 0e588ab4-9dcf-473f-84dc-f38df7df499b
📒 Files selected for processing (18)
docs/devel_doc/openapi.jsondocs/user_doc/a2a_protocol.mdexamples/lightspeed-stack-a2a-agents.yamlsrc/a2a_client/__init__.pysrc/a2a_client/capability.pysrc/a2a_client/manager.pysrc/app/main.pysrc/configuration.pysrc/models/config.pysrc/telemetry/configuration_snapshot.pysrc/utils/pydantic_ai_helpers.pytests/unit/a2a_client/__init__.pytests/unit/a2a_client/test_capability.pytests/unit/a2a_client/test_manager.pytests/unit/models/config/test_dump_configuration.pytests/unit/telemetry/conftest.pytests/unit/telemetry/test_configuration_snapshot.pytests/unit/telemetry/test_configuration_snapshot_new_sections.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
- GitHub Check: spectral
- GitHub Check: shellcheck
- GitHub Check: integration_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: Pylinter
- GitHub Check: radon
- GitHub Check: unit_tests (3.13)
- GitHub Check: unit_tests (3.12)
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: build-pr
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pyexamples/lightspeed-stack-a2a-agents.yamldocs/devel_doc/openapi.jsonsrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/models/config.pydocs/user_doc/a2a_protocol.mdtests/unit/models/config/test_dump_configuration.pysrc/a2a_client/manager.pysrc/a2a_client/capability.pytests/unit/a2a_client/test_manager.pytests/unit/telemetry/test_configuration_snapshot_new_sections.pytests/unit/a2a_client/test_capability.pysrc/telemetry/configuration_snapshot.pytests/unit/telemetry/conftest.pytests/unit/telemetry/test_configuration_snapshot.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings explaining their purpose.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
Use complete type annotations for function parameters and return types.
Use modern union syntax such asstr | int; useOptional[Type]for optional values.
Usetyping_extensions.Selffor model validators.
Functions must use descriptive, action-oriented snake_case names such asget_,validate_, andcheck_.
Avoid modifying mutable parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack.
All classes must have descriptive docstrings and complete type annotations for class attributes; use specific types instead ofAny.
Use PascalCase for classes and descriptive standard suffixes such asConfiguration,Error/Exception,Resolver, andInterface.
Use ABC and@abstractmethodfor abstract interfaces.
Follow Google Python docstring conventions; document all modules, classes, and functions, includingParameters,Returns,Raises, andAttributessections as applicable.
Never commit secrets or keys; use environment variables for sensitive data.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pysrc/a2a_client/manager.pysrc/a2a_client/capability.pytests/unit/a2a_client/test_manager.pytests/unit/telemetry/test_configuration_snapshot_new_sections.pytests/unit/a2a_client/test_capability.pysrc/telemetry/configuration_snapshot.pytests/unit/telemetry/conftest.pytests/unit/telemetry/test_configuration_snapshot.py
**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Package
__init__.pyfiles must contain brief package descriptions.
Files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/unit/**/*.py: Use pytest for all unit tests; do not use unittest.
Useconftest.pyfor shared fixtures,pytest-mockforAsyncMockobjects, andpytest.mark.asynciofor asynchronous tests.
Files:
tests/unit/a2a_client/__init__.pytests/unit/models/config/test_dump_configuration.pytests/unit/a2a_client/test_manager.pytests/unit/telemetry/test_configuration_snapshot_new_sections.pytests/unit/a2a_client/test_capability.pytests/unit/telemetry/conftest.pytests/unit/telemetry/test_configuration_snapshot.py
src/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/models/**/*.py: ExtendConfigurationBasefor configuration models andBaseModelfor data models.
Use@model_validatorand@field_validatorfor Pydantic model validation.
Files:
src/models/config.py
🧠 Learnings (7)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/models/config.pytests/unit/models/config/test_dump_configuration.pysrc/a2a_client/manager.pysrc/a2a_client/capability.pytests/unit/a2a_client/test_manager.pytests/unit/telemetry/test_configuration_snapshot_new_sections.pytests/unit/a2a_client/test_capability.pysrc/telemetry/configuration_snapshot.pytests/unit/telemetry/conftest.pytests/unit/telemetry/test_configuration_snapshot.py
📚 Learning: 2026-07-21T11:10:05.060Z
Learnt from: are-ces
Repo: lightspeed-core/lightspeed-stack PR: 2162
File: src/a2a_client/__init__.py:3-9
Timestamp: 2026-07-21T11:10:05.060Z
Learning: In this repository, it is acceptable for Python package `__init__.py` files to contain functional code (not only docstrings/metadata) and to perform package-level re-exports. Do not flag `__init__.py` solely for containing imports or other logic used to re-export symbols; this is allowed when it’s implemented via imports and `__all__` (or otherwise clearly intended to define the package’s public API).
Applied to files:
tests/unit/a2a_client/__init__.pysrc/a2a_client/__init__.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.
Applied to files:
src/a2a_client/__init__.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/models/config.pysrc/a2a_client/manager.pysrc/a2a_client/capability.pysrc/telemetry/configuration_snapshot.py
📚 Learning: 2026-07-17T19:25:05.325Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2166
File: src/utils/saved_prompts.py:129-157
Timestamp: 2026-07-17T19:25:05.325Z
Learning: For any endpoint that handles saved prompts and calls `src/utils/saved_prompts.py::create_saved_prompt`, treat the endpoint as the validation boundary. Before calling `create_saved_prompt`, validate the incoming saved-prompt name and content, specifically using `validate_saved_prompt_name` and then persist (store) the normalized value it returns. Do not call `create_saved_prompt` with unvalidated/raw name/content.
Applied to files:
src/a2a_client/__init__.pysrc/app/main.pysrc/utils/pydantic_ai_helpers.pysrc/configuration.pysrc/models/config.pysrc/a2a_client/manager.pysrc/a2a_client/capability.pysrc/telemetry/configuration_snapshot.py
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
examples/lightspeed-stack-a2a-agents.yaml
📚 Learning: 2026-01-12T10:58:40.230Z
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:40.230Z
Learning: In lightspeed-core/lightspeed-stack, for Python files under src/models, when a user claims a fix is done but the issue persists, verify the current code state before accepting the fix. Steps: review the diff, fetch the latest changes, run relevant tests, reproduce the issue, search the codebase for lingering references to the original problem, confirm the fix is applied and not undone by subsequent commits, and validate with local checks to ensure the issue is resolved.
Applied to files:
src/models/config.py
📚 Learning: 2026-02-25T07:46:33.545Z
Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:33.545Z
Learning: In the Python codebase, requests.py should use OpenAIResponseInputTool as Tool while responses.py uses OpenAIResponseTool as Tool. This difference is intentional due to differing schemas for input vs output tools in llama-stack-api. Apply this distinction consistently to other models under src/models (e.g., ensure request-related tools use the InputTool variant and response-related tools use the ResponseTool variant). If adding new tools, choose the corresponding InputTool or Tool class based on whether the tool represents input or output, and document the rationale in code comments.
Applied to files:
src/models/config.py
🪛 ast-grep (0.45.0)
tests/unit/telemetry/test_configuration_snapshot_new_sections.py
[info] 902-902: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 908-908: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 923-923: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 938-938: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 952-952: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 969-969: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 976-976: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 987-987: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 994-994: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1000-1000: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1005-1005: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1011-1011: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1022-1022: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1049-1049: use jsonify instead of json.dumps for JSON output
Context: json.dumps(minimal_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1057-1057: use jsonify instead of json.dumps for JSON output
Context: json.dumps(full_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 1078-1078: use jsonify instead of json.dumps for JSON output
Context: json.dumps(minimal_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Checkov (3.3.8)
docs/devel_doc/openapi.json
[medium] 10931-10938: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🔇 Additional comments (31)
src/models/config.py (1)
2025-2034: LGTM!Also applies to: 2041-2062, 2771-2775
docs/devel_doc/openapi.json (1)
10877-10944: LGTM!Also applies to: 12393-12404
tests/unit/models/config/test_dump_configuration.py (1)
212-212: LGTM!Also applies to: 436-436, 811-811, 1070-1070, 1309-1309, 1528-1528, 1907-1907, 2132-2132, 2357-2357, 2589-2589
docs/user_doc/a2a_protocol.md (1)
7-10: LGTM!Also applies to: 876-878
src/a2a_client/manager.py (1)
135-163: LGTM!tests/unit/a2a_client/test_manager.py (1)
56-60: LGTM!Also applies to: 171-192
src/app/main.py (1)
136-142: 🎯 Functional CorrectnessConfiguration accessor is valid.
examples/lightspeed-stack-a2a-agents.yaml (1)
9-41: LGTM!src/a2a_client/__init__.py (1)
1-9: LGTM!src/a2a_client/capability.py (3)
27-40: LGTM!
43-58: LGTM!
117-148: LGTM!tests/unit/a2a_client/test_capability.py (1)
154-393: LGTM!src/utils/pydantic_ai_helpers.py (1)
14-15: LGTM!Also applies to: 133-142, 161-162
tests/unit/a2a_client/__init__.py (1)
1-1: LGTM!tests/unit/telemetry/test_configuration_snapshot.py (3)
3-4: Blankettoo-many-public-methodsdisable is a symptom of the class growth flagged below at Lines 499-956; nothing extra to fix here.
192-194: LGTM!
453-456: LGTM!src/telemetry/configuration_snapshot.py (5)
82-86: LGTM!
99-124: LGTM!
146-177: LGTM!
186-189: LGTM!
201-333: LGTM!tests/unit/telemetry/conftest.py (4)
11-54: LGTM!
86-134: LGTM!
161-209: LGTM!
305-593: LGTM!tests/unit/telemetry/test_configuration_snapshot_new_sections.py (4)
83-100: LGTM!
111-129: LGTM!
409-435: LGTM!Also applies to: 516-571, 785-805
901-1027: LGTM!
Comment on lines
+10930
to
+10939
| "properties": { | ||
| "agents": { | ||
| "items": { | ||
| "$ref": "#/components/schemas/A2AAgentEndpointConfiguration" | ||
| }, | ||
| "type": "array", | ||
| "title": "A2A agent endpoints", | ||
| "description": "External A2A agents available for task delegation." | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
Unbounded agents array flagged by static analysis.
Checkov (CKV_OPENAPI_21) flags the agents array here for missing maxItems. Since this file is auto-generated from the Pydantic config models, the fix belongs on the source A2AAgentsConfiguration.agents field (e.g., a max_length constraint), not in this JSON directly. Given a2a_agents is server-side config only surfaced via GET /v1/config, exploitability is low, but bounding it is still a cheap hardening step.
🧰 Tools
🪛 Checkov (3.3.8)
[medium] 10931-10938: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🤖 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/devel_doc/openapi.json` around lines 10930 - 10939, Bound the A2A agents
collection at the source model by adding an appropriate max-length constraint to
the A2AAgentsConfiguration.agents field, then regenerate
docs/devel_doc/openapi.json so the generated schema includes maxItems. Do not
edit the generated OpenAPI JSON directly.
Source: Linters/SAST tools
Comment on lines
+316
to
+317
| 3. **Builds Pydantic-AI Agent**: Uses `build_agent()` with the same capabilities as the query/streaming endpoints (skills, A2A delegation) | ||
| 4. **Streams via Agent**: Runs `agent.run_stream_events()` and converts pydantic-ai events to A2A events |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the executor flow diagrams.
The updated Pydantic-AI event flow conflicts with the retained direct “Llama Stack Responses API” flow at Lines 337-344. Update the diagrams or explain the adapter boundary.
🤖 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/user_doc/a2a_protocol.md` around lines 316 - 317, Align the executor
flow described around build_agent() and run_stream_events() with the retained
direct “Llama Stack Responses API” flow, either by updating both diagrams to
show the same execution path or by explicitly documenting the adapter boundary
between them. Ensure the final documentation consistently explains how
Pydantic-AI events map to the A2A/Responses API events.
| # A2A-compliant agents. The LLM decides when to delegate based on agent | ||
| # descriptions from their agent cards. | ||
| # | ||
| # See docs/a2a_protocol.md for details. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale doc path. The A2A protocol doc in this PR is docs/user_doc/a2a_protocol.md.
-# See docs/a2a_protocol.md for details.
+# See docs/user_doc/a2a_protocol.md for details.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # See docs/a2a_protocol.md for details. | |
| # See docs/user_doc/a2a_protocol.md for details. |
🤖 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 `@examples/lightspeed-stack-a2a-agents.yaml` at line 7, Update the
documentation reference in the comment at the top of the Lightspeed A2A agents
configuration to point to docs/user_doc/a2a_protocol.md instead of the stale
docs/a2a_protocol.md path.
Comment on lines
+88
to
+91
| Args: | ||
| agent_name: Name of the agent to delegate to (from list_agents). | ||
| task: The task description or question to send to the agent. | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use Parameters: instead of Args:.
The sibling helpers in this file already use Parameters:.
- Args:
+ Parameters:
agent_name: Name of the agent to delegate to (from list_agents).
task: The task description or question to send to the agent.Based on learnings: docstrings in this repo must use the section header "Parameters:" (not "Args:") for function arguments.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Args: | |
| agent_name: Name of the agent to delegate to (from list_agents). | |
| task: The task description or question to send to the agent. | |
| Parameters: | |
| agent_name: Name of the agent to delegate to (from list_agents). | |
| task: The task description or question to send to the agent. |
🤖 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 `@src/a2a_client/capability.py` around lines 88 - 91, Update the docstring for
the affected capability helper to rename the argument section header from
“Args:” to “Parameters:”, matching the convention used by sibling helpers in the
same file; leave the parameter descriptions unchanged.
Source: Learnings
Comment on lines
+102
to
+115
| try: | ||
| logger.debug("Delegating task to agent '%s'", agent_name) | ||
| message = create_text_message_object(role=Role.user, content=task) | ||
| return await _send_and_collect(client, message) | ||
| except A2AClientError as e: | ||
| logger.warning( | ||
| "A2A delegation to '%s' failed: %s", | ||
| agent_name, | ||
| e, | ||
| ) | ||
| return ( | ||
| f"Delegation to agent '{agent_name}' failed: {e}. " | ||
| "Please try answering directly or inform the user." | ||
| ) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Broaden the exception guard: transport errors escape the tool.
A2AClientManager.initialize (src/a2a_client/manager.py:83-133) guards (A2AClientError, httpx.HTTPError, OSError) for the same client, so the SDK does leak raw transport errors. A mid-stream httpx.ReadTimeout/ConnectError here propagates out of the tool and aborts the agent run instead of returning the graceful degradation string.
🛡️ Proposed fix
+import httpx
+
...
try:
logger.debug("Delegating task to agent '%s'", agent_name)
message = create_text_message_object(role=Role.user, content=task)
return await _send_and_collect(client, message)
- except A2AClientError as e:
+ except (A2AClientError, httpx.HTTPError, OSError) as e:
logger.warning(
"A2A delegation to '%s' failed: %s",
agent_name,
e,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| logger.debug("Delegating task to agent '%s'", agent_name) | |
| message = create_text_message_object(role=Role.user, content=task) | |
| return await _send_and_collect(client, message) | |
| except A2AClientError as e: | |
| logger.warning( | |
| "A2A delegation to '%s' failed: %s", | |
| agent_name, | |
| e, | |
| ) | |
| return ( | |
| f"Delegation to agent '{agent_name}' failed: {e}. " | |
| "Please try answering directly or inform the user." | |
| ) | |
| try: | |
| logger.debug("Delegating task to agent '%s'", agent_name) | |
| message = create_text_message_object(role=Role.user, content=task) | |
| return await _send_and_collect(client, message) | |
| except (A2AClientError, httpx.HTTPError, OSError) as e: | |
| logger.warning( | |
| "A2A delegation to '%s' failed: %s", | |
| agent_name, | |
| e, | |
| ) | |
| return ( | |
| f"Delegation to agent '{agent_name}' failed: {e}. " | |
| "Please try answering directly or inform the user." | |
| ) |
🤖 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 `@src/a2a_client/capability.py` around lines 102 - 115, Broaden the exception
handler around message creation and _send_and_collect in the delegation flow to
catch A2AClientError, httpx.HTTPError, and OSError, matching the guard used by
A2AClientManager.initialize. Preserve the existing warning and graceful
degradation response, including the agent name and error details, for all
handled transport failures.
Comment on lines
672
to
685
| conversation_cache=None, | ||
| compaction=CompactionConfiguration.model_construct( | ||
| enabled=False, | ||
| threshold_ratio=0.7, | ||
| token_floor=4096, | ||
| buffer_turns=4, | ||
| buffer_max_ratio=0.3, | ||
| ), | ||
| byok_rag=[], | ||
| a2a_state=None, | ||
| a2a_agents=None, | ||
| quota_handlers=None, | ||
| azure_entra_id=None, | ||
| splunk=None, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Minimal fixture encodes a shape the real model can't produce.
conversation_cache, a2a_state, and quota_handlers are non-Optional on Configuration with default_factory (see src/models/config.py), so a loaded config always has instances there — never None. Setting them to None here means the "minimal" assertions in the two test modules never exercise the actual default shape (they pass either way, since empty sub-configs also mask to NOT_CONFIGURED).
Prefer the real defaults so the fixture stays faithful:
♻️ Suggested change
- conversation_cache=None,
+ conversation_cache=ConversationHistoryConfiguration.model_construct(
+ type=None, memory=None, sqlite=None, postgres=None
+ ),
@@
- a2a_state=None,
+ a2a_state=A2AStateConfiguration.model_construct(sqlite=None, postgres=None),
a2a_agents=None,
- quota_handlers=None,
+ quota_handlers=QuotaHandlersConfiguration.model_construct(
+ sqlite=None,
+ postgres=None,
+ limiters=[],
+ scheduler=QuotaSchedulerConfiguration.model_construct(
+ period=1,
+ database_reconnection_count=10,
+ database_reconnection_delay=1,
+ ),
+ enable_token_history=False,
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| conversation_cache=None, | |
| compaction=CompactionConfiguration.model_construct( | |
| enabled=False, | |
| threshold_ratio=0.7, | |
| token_floor=4096, | |
| buffer_turns=4, | |
| buffer_max_ratio=0.3, | |
| ), | |
| byok_rag=[], | |
| a2a_state=None, | |
| a2a_agents=None, | |
| quota_handlers=None, | |
| azure_entra_id=None, | |
| splunk=None, | |
| conversation_cache=ConversationHistoryConfiguration.model_construct( | |
| type=None, memory=None, sqlite=None, postgres=None | |
| ), | |
| compaction=CompactionConfiguration.model_construct( | |
| enabled=False, | |
| threshold_ratio=0.7, | |
| token_floor=4096, | |
| buffer_turns=4, | |
| buffer_max_ratio=0.3, | |
| ), | |
| byok_rag=[], | |
| a2a_state=A2AStateConfiguration.model_construct(sqlite=None, postgres=None), | |
| a2a_agents=None, | |
| quota_handlers=QuotaHandlersConfiguration.model_construct( | |
| sqlite=None, | |
| postgres=None, | |
| limiters=[], | |
| scheduler=QuotaSchedulerConfiguration.model_construct( | |
| period=1, | |
| database_reconnection_count=10, | |
| database_reconnection_delay=1, | |
| ), | |
| enable_token_history=False, | |
| ), | |
| azure_entra_id=None, | |
| splunk=None, |
🤖 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/unit/telemetry/conftest.py` around lines 672 - 685, Update the minimal
configuration fixture’s conversation_cache, a2a_state, and quota_handlers fields
to use the same default-factory-created instances as Configuration instead of
None. Keep the fixture otherwise unchanged so tests exercise the model’s real
default shape.
Comment on lines
+391
to
+398
| def test_a2a_state_not_configured_when_none(self, minimal_snapshot: dict) -> None: | ||
| """Test a2a_state shows not_configured when None.""" | ||
| a2a = minimal_snapshot.get("a2a_state") | ||
| if a2a is not None: | ||
| assert a2a.get("sqlite", {}).get("db_path") in ( | ||
| None, | ||
| NOT_CONFIGURED, | ||
| ) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Conditional guards make these *_not_configured tests near-no-ops.
The registry always writes every top-level section key, so minimal_snapshot.get("a2a_state") is never None and the guard is dead. Accepting both None and NOT_CONFIGURED further weakens the check. Same pattern at Lines 197-202 (conversation_cache), 275-283 (quota_handlers), 437-441 (splunk), 601-605 (azure_entra_id).
Assert exactly, as the sibling module already does:
♻️ Suggested change
def test_a2a_state_not_configured_when_none(self, minimal_snapshot: dict) -> None:
- """Test a2a_state shows not_configured when None."""
- a2a = minimal_snapshot.get("a2a_state")
- if a2a is not None:
- assert a2a.get("sqlite", {}).get("db_path") in (
- None,
- NOT_CONFIGURED,
- )
+ """Test a2a_state shows not_configured when unset."""
+ assert minimal_snapshot["a2a_state"]["sqlite"]["db_path"] == NOT_CONFIGURED
+ assert minimal_snapshot["a2a_state"]["postgres"]["host"] == NOT_CONFIGURED📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_a2a_state_not_configured_when_none(self, minimal_snapshot: dict) -> None: | |
| """Test a2a_state shows not_configured when None.""" | |
| a2a = minimal_snapshot.get("a2a_state") | |
| if a2a is not None: | |
| assert a2a.get("sqlite", {}).get("db_path") in ( | |
| None, | |
| NOT_CONFIGURED, | |
| ) | |
| def test_a2a_state_not_configured_when_none(self, minimal_snapshot: dict) -> None: | |
| """Test a2a_state shows not_configured when unset.""" | |
| assert minimal_snapshot["a2a_state"]["sqlite"]["db_path"] == NOT_CONFIGURED | |
| assert minimal_snapshot["a2a_state"]["postgres"]["host"] == NOT_CONFIGURED |
🤖 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/unit/telemetry/test_configuration_snapshot_new_sections.py` around
lines 391 - 398, Strengthen the *_not_configured tests by removing conditional
guards and asserting the expected NOT_CONFIGURED value directly. Update
test_a2a_state_not_configured_when_none and the equivalent conversation_cache,
quota_handlers, splunk, and azure_entra_id tests to access their sections
unconditionally and require the documented exact value, following the sibling
module’s assertion pattern.
Comment on lines
+816
to
+882
| @staticmethod | ||
| def _registry_paths() -> set[str]: | ||
| """Extract all top-level section names from the field registry.""" | ||
| paths: set[str] = set() | ||
| for spec in LIGHTSPEED_STACK_FIELDS: | ||
| if isinstance(spec, FieldSpec): | ||
| paths.add(spec.path.split(".")[0]) | ||
| elif isinstance(spec, ListFieldSpec): | ||
| paths.add(spec.path.split(".")[0]) | ||
| return paths | ||
|
|
||
| @pytest.mark.parametrize( | ||
| "section", | ||
| [ | ||
| "compaction", | ||
| "conversation_cache", | ||
| "quota_handlers", | ||
| "byok_rag", | ||
| "a2a_state", | ||
| "splunk", | ||
| "inference", | ||
| "llama_stack", | ||
| "authentication", | ||
| "azure_entra_id", | ||
| "customization", | ||
| "okp", | ||
| "rag", | ||
| "reranker", | ||
| "approvals", | ||
| "rlsapi_v1", | ||
| "saved_prompts", | ||
| "skills", | ||
| "deployment_environment", | ||
| "mcp_servers", | ||
| ], | ||
| ) | ||
| def test_section_in_registry(self, section: str) -> None: | ||
| """Test that each new section has at least one field in the registry.""" | ||
| paths = self._registry_paths() | ||
| assert ( | ||
| section in paths | ||
| ), f"Section '{section}' not found in LIGHTSPEED_STACK_FIELDS registry" | ||
|
|
||
| @pytest.mark.parametrize( | ||
| "section", | ||
| [ | ||
| "compaction", | ||
| "conversation_cache", | ||
| "quota_handlers", | ||
| "byok_rag", | ||
| "a2a_state", | ||
| "splunk", | ||
| "inference", | ||
| "llama_stack", | ||
| "authentication", | ||
| "azure_entra_id", | ||
| "customization", | ||
| "okp", | ||
| "rag", | ||
| "reranker", | ||
| "approvals", | ||
| "rlsapi_v1", | ||
| "saved_prompts", | ||
| "skills", | ||
| "deployment_environment", | ||
| "mcp_servers", | ||
| ], |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Hoist the section list and collapse the identical branches.
The 20-element list is duplicated verbatim across both parametrize decorators, and _registry_paths (also re-implemented inline at Lines 1066-1071) takes the same action in both branches.
♻️ Suggested change
+NEW_SECTIONS = (
+ "compaction",
+ "conversation_cache",
+ # ... remaining sections
+)
+
+
+def _registry_top_level_paths() -> set[str]:
+ """Return all top-level section names in the field registry."""
+ return {spec.path.split(".")[0] for spec in LIGHTSPEED_STACK_FIELDS}Then @pytest.mark.parametrize("section", NEW_SECTIONS) on both tests and reuse _registry_top_level_paths() in test_snapshot_only_contains_allowlisted_fields.
🤖 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/unit/telemetry/test_configuration_snapshot_new_sections.py` around
lines 816 - 882, Define a shared NEW_SECTIONS constant for the 20 section names
and use it in both test_section_in_registry and the other parametrized test
instead of duplicating the list. Consolidate the duplicated registry-path
extraction into _registry_top_level_paths() and update
test_snapshot_only_contains_allowlisted_fields to reuse that helper in both
branches.
Comment on lines
+1045
to
+1052
| def test_none_optional_sections_produce_not_configured( | ||
| self, minimal_snapshot: dict | ||
| ) -> None: | ||
| """Test that None optional sections produce not_configured markers.""" | ||
| # These sections are None in minimal config | ||
| json_str = json.dumps(minimal_snapshot) | ||
| # The snapshot should be JSON-serializable even with None sections | ||
| assert isinstance(json.loads(json_str), dict) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test name promises not_configured coverage it doesn't provide.
The body only checks JSON round-tripping — identical to test_snapshot_is_json_serializable_with_all_sections below. Either assert the actual markers or rename.
🐛 Suggested fix
- def test_none_optional_sections_produce_not_configured(
- self, minimal_snapshot: dict
- ) -> None:
- """Test that None optional sections produce not_configured markers."""
- # These sections are None in minimal config
- json_str = json.dumps(minimal_snapshot)
- # The snapshot should be JSON-serializable even with None sections
- assert isinstance(json.loads(json_str), dict)
+ def test_none_optional_sections_produce_not_configured(
+ self, minimal_snapshot: dict
+ ) -> None:
+ """Test that unset optional sections produce not_configured markers."""
+ assert minimal_snapshot["azure_entra_id"]["tenant_id"] == NOT_CONFIGURED
+ assert minimal_snapshot["splunk"]["url"] == NOT_CONFIGURED
+ assert minimal_snapshot["skills"]["paths"] == NOT_CONFIGURED
+ assert minimal_snapshot["a2a_agents"]["agents"] == NOT_CONFIGURED📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_none_optional_sections_produce_not_configured( | |
| self, minimal_snapshot: dict | |
| ) -> None: | |
| """Test that None optional sections produce not_configured markers.""" | |
| # These sections are None in minimal config | |
| json_str = json.dumps(minimal_snapshot) | |
| # The snapshot should be JSON-serializable even with None sections | |
| assert isinstance(json.loads(json_str), dict) | |
| def test_none_optional_sections_produce_not_configured( | |
| self, minimal_snapshot: dict | |
| ) -> None: | |
| """Test that unset optional sections produce not_configured markers.""" | |
| assert minimal_snapshot["azure_entra_id"]["tenant_id"] == NOT_CONFIGURED | |
| assert minimal_snapshot["splunk"]["url"] == NOT_CONFIGURED | |
| assert minimal_snapshot["skills"]["paths"] == NOT_CONFIGURED | |
| assert minimal_snapshot["a2a_agents"]["agents"] == NOT_CONFIGURED |
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 1049-1049: use jsonify instead of json.dumps for JSON output
Context: json.dumps(minimal_snapshot)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 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/unit/telemetry/test_configuration_snapshot_new_sections.py` around
lines 1045 - 1052, Update test_none_optional_sections_produce_not_configured to
assert that each optional section set to None produces the expected
not_configured markers in the snapshot output, rather than only checking JSON
serialization. Reuse the existing snapshot structure and marker symbols used by
the surrounding tests, while preserving the serialization assertion if still
relevant.
Comment on lines
+499
to
+956
| def test_service_base_url_masked(self) -> None: | ||
| """Test service base_url is masked as sensitive.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["service"]["base_url"] == CONFIGURED | ||
|
|
||
| def test_service_base_url_none(self) -> None: | ||
| """Test service base_url when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["service"]["base_url"] == NOT_CONFIGURED | ||
|
|
||
| def test_service_root_path_masked(self) -> None: | ||
| """Test service root_path is masked as sensitive.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["service"]["root_path"] == CONFIGURED | ||
|
|
||
| def test_llama_stack_timeout_passthrough(self) -> None: | ||
| """Test llama_stack timeout passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["llama_stack"]["timeout"] == 180 | ||
|
|
||
| def test_llama_stack_max_retries_passthrough(self) -> None: | ||
| """Test llama_stack max_retries passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["llama_stack"]["max_retries"] == 5 | ||
|
|
||
| def test_llama_stack_retry_delay_passthrough(self) -> None: | ||
| """Test llama_stack retry_delay passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["llama_stack"]["retry_delay"] == 2 | ||
|
|
||
| def test_llama_stack_allow_degraded_mode_passthrough(self) -> None: | ||
| """Test llama_stack allow_degraded_mode passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["llama_stack"]["allow_degraded_mode"] is True | ||
|
|
||
| def test_llama_stack_config_baseline_passthrough(self) -> None: | ||
| """Test llama_stack config baseline passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["llama_stack"]["config"]["baseline"] == "default" | ||
|
|
||
| def test_llama_stack_config_profile_masked(self) -> None: | ||
| """Test llama_stack config profile is masked as sensitive.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["llama_stack"]["config"]["profile"] == CONFIGURED | ||
|
|
||
| def test_llama_stack_config_native_override_masked(self) -> None: | ||
| """Test llama_stack config native_override is masked as sensitive.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["llama_stack"]["config"]["native_override"] == CONFIGURED | ||
|
|
||
| def test_llama_stack_config_none(self) -> None: | ||
| """Test llama_stack config fields when config is None.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["llama_stack"]["config"]["baseline"] is None | ||
| assert snapshot["llama_stack"]["config"]["profile"] == NOT_CONFIGURED | ||
| assert snapshot["llama_stack"]["config"]["native_override"] == NOT_CONFIGURED | ||
|
|
||
| def test_inference_context_windows_passthrough(self) -> None: | ||
| """Test inference context_windows passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["inference"]["context_windows"] == { | ||
| "openai/gpt-4o-mini": 128000 | ||
| } | ||
|
|
||
| def test_inference_max_infer_iters_passthrough(self) -> None: | ||
| """Test inference max_infer_iters passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["inference"]["max_infer_iters"] == 10 | ||
|
|
||
| def test_inference_max_tool_calls_passthrough(self) -> None: | ||
| """Test inference max_tool_calls passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["inference"]["max_tool_calls"] == 30 | ||
|
|
||
| def test_inference_providers_extraction(self) -> None: | ||
| """Test inference providers list extraction with masking.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| providers = snapshot["inference"]["providers"] | ||
| assert isinstance(providers, list) | ||
| assert len(providers) == 1 | ||
| assert providers[0]["type"] == "openai" | ||
| assert providers[0]["id"] == "openai-provider" | ||
| assert providers[0]["api_key_env"] == CONFIGURED | ||
| assert providers[0]["allowed_models"] == ["gpt-4o-mini", "gpt-4o"] | ||
|
|
||
| def test_inference_providers_empty(self) -> None: | ||
| """Test inference providers when empty.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["inference"]["providers"] == [] | ||
|
|
||
| def test_authentication_skip_for_health_probes(self) -> None: | ||
| """Test authentication skip_for_health_probes passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["authentication"]["skip_for_health_probes"] is True | ||
|
|
||
| def test_authentication_skip_for_metrics(self) -> None: | ||
| """Test authentication skip_for_metrics passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["authentication"]["skip_for_metrics"] is True | ||
|
|
||
| def test_authentication_api_key_config_masked(self) -> None: | ||
| """Test authentication api_key_config.api_key is masked.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["authentication"]["api_key_config"]["api_key"] == CONFIGURED | ||
|
|
||
| def test_authentication_api_key_config_none(self) -> None: | ||
| """Test authentication api_key_config when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["authentication"]["api_key_config"]["api_key"] == NOT_CONFIGURED | ||
|
|
||
| def test_authentication_rh_identity_config(self) -> None: | ||
| """Test authentication rh_identity_config fields.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert ( | ||
| snapshot["authentication"]["rh_identity_config"]["required_entitlements"] | ||
| == CONFIGURED | ||
| ) | ||
| assert ( | ||
| snapshot["authentication"]["rh_identity_config"]["max_header_size"] == 16384 | ||
| ) | ||
|
|
||
| def test_authentication_rh_identity_config_none(self) -> None: | ||
| """Test authentication rh_identity_config when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert ( | ||
| snapshot["authentication"]["rh_identity_config"]["required_entitlements"] | ||
| == NOT_CONFIGURED | ||
| ) | ||
| assert ( | ||
| snapshot["authentication"]["rh_identity_config"]["max_header_size"] is None | ||
| ) | ||
|
|
||
| def test_authentication_trusted_proxy_config(self) -> None: | ||
| """Test authentication trusted_proxy_config fields.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert ( | ||
| snapshot["authentication"]["trusted_proxy_config"]["user_header"] | ||
| == "X-Forwarded-User" | ||
| ) | ||
| accounts = snapshot["authentication"]["trusted_proxy_config"][ | ||
| "allowed_service_accounts" | ||
| ] | ||
| assert isinstance(accounts, list) | ||
| assert len(accounts) == 1 | ||
| assert accounts[0]["namespace"] == CONFIGURED | ||
| assert accounts[0]["name"] == CONFIGURED | ||
|
|
||
| def test_authentication_trusted_proxy_config_none(self) -> None: | ||
| """Test authentication trusted_proxy_config when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["authentication"]["trusted_proxy_config"]["user_header"] is None | ||
| assert ( | ||
| snapshot["authentication"]["trusted_proxy_config"][ | ||
| "allowed_service_accounts" | ||
| ] | ||
| == NOT_CONFIGURED | ||
| ) | ||
|
|
||
| def test_azure_entra_id_fields(self) -> None: | ||
| """Test azure_entra_id fields are properly masked.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["azure_entra_id"]["tenant_id"] == CONFIGURED | ||
| assert snapshot["azure_entra_id"]["client_id"] == CONFIGURED | ||
| assert snapshot["azure_entra_id"]["client_secret"] == CONFIGURED | ||
| assert ( | ||
| snapshot["azure_entra_id"]["scope"] | ||
| == "https://cognitiveservices.azure.com/.default" | ||
| ) | ||
|
|
||
| def test_azure_entra_id_none(self) -> None: | ||
| """Test azure_entra_id when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["azure_entra_id"]["tenant_id"] == NOT_CONFIGURED | ||
| assert snapshot["azure_entra_id"]["client_id"] == NOT_CONFIGURED | ||
| assert snapshot["azure_entra_id"]["client_secret"] == NOT_CONFIGURED | ||
| assert snapshot["azure_entra_id"]["scope"] is None | ||
|
|
||
| def test_customization_profile_path_masked(self) -> None: | ||
| """Test customization profile_path is masked as sensitive.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["customization"]["profile_path"] == CONFIGURED | ||
|
|
||
| def test_customization_disable_shield_ids_override(self) -> None: | ||
| """Test customization disable_shield_ids_override passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["customization"]["disable_shield_ids_override"] is True | ||
|
|
||
| def test_customization_agent_card_path_masked(self) -> None: | ||
| """Test customization agent_card_path is masked as sensitive.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["customization"]["agent_card_path"] == CONFIGURED | ||
|
|
||
| def test_conversation_cache_fields(self) -> None: | ||
| """Test conversation_cache fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| cache = snapshot["conversation_cache"] | ||
| assert cache["type"] == "postgres" | ||
| assert cache["memory"]["max_entries"] == 1000 | ||
| assert cache["sqlite"]["db_path"] == CONFIGURED | ||
| assert cache["postgres"]["host"] == CONFIGURED | ||
| assert cache["postgres"]["port"] == 5432 | ||
| assert cache["postgres"]["db"] == CONFIGURED | ||
| assert cache["postgres"]["user"] == CONFIGURED | ||
| assert cache["postgres"]["password"] == CONFIGURED | ||
| assert cache["postgres"]["namespace"] == CONFIGURED | ||
| assert cache["postgres"]["ssl_mode"] == "verify-full" | ||
| assert cache["postgres"]["gss_encmode"] == "prefer" | ||
| assert cache["postgres"]["ca_cert_path"] == CONFIGURED | ||
|
|
||
| def test_conversation_cache_none(self) -> None: | ||
| """Test conversation_cache when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["conversation_cache"]["type"] is None | ||
| assert snapshot["conversation_cache"]["memory"]["max_entries"] is None | ||
| assert snapshot["conversation_cache"]["sqlite"]["db_path"] == NOT_CONFIGURED | ||
| assert snapshot["conversation_cache"]["postgres"]["host"] == NOT_CONFIGURED | ||
|
|
||
| def test_compaction_fields(self) -> None: | ||
| """Test compaction fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| compaction = snapshot["compaction"] | ||
| assert compaction["enabled"] is True | ||
| assert compaction["threshold_ratio"] == 0.8 | ||
| assert compaction["token_floor"] == 8192 | ||
| assert compaction["buffer_turns"] == 6 | ||
| assert compaction["buffer_max_ratio"] == 0.4 | ||
|
|
||
| def test_compaction_defaults(self) -> None: | ||
| """Test compaction fields with default values.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| compaction = snapshot["compaction"] | ||
| assert compaction["enabled"] is False | ||
| assert compaction["threshold_ratio"] == 0.7 | ||
| assert compaction["token_floor"] == 4096 | ||
| assert compaction["buffer_turns"] == 4 | ||
| assert compaction["buffer_max_ratio"] == 0.3 | ||
|
|
||
| def test_quota_handlers_fields(self) -> None: | ||
| """Test quota_handlers fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| qh = snapshot["quota_handlers"] | ||
| assert qh["sqlite"]["db_path"] == CONFIGURED | ||
| assert qh["postgres"]["host"] == CONFIGURED | ||
| assert qh["postgres"]["port"] == 5432 | ||
| assert qh["postgres"]["db"] == CONFIGURED | ||
| assert qh["postgres"]["user"] == CONFIGURED | ||
| assert qh["postgres"]["password"] == CONFIGURED | ||
| assert qh["postgres"]["namespace"] == CONFIGURED | ||
| assert qh["postgres"]["ssl_mode"] == "verify-full" | ||
| assert qh["postgres"]["gss_encmode"] == "prefer" | ||
| assert qh["postgres"]["ca_cert_path"] == CONFIGURED | ||
| assert qh["enable_token_history"] is True | ||
|
|
||
| def test_quota_handlers_limiters(self) -> None: | ||
| """Test quota_handlers limiters list extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| limiters = snapshot["quota_handlers"]["limiters"] | ||
| assert isinstance(limiters, list) | ||
| assert len(limiters) == 1 | ||
| assert limiters[0]["type"] == "user_limiter" | ||
| assert limiters[0]["name"] == "daily-user-limit" | ||
| assert limiters[0]["initial_quota"] == 10000 | ||
| assert limiters[0]["quota_increase"] == 0 | ||
| assert limiters[0]["period"] == "1 day" | ||
|
|
||
| def test_quota_handlers_scheduler(self) -> None: | ||
| """Test quota_handlers scheduler fields.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| scheduler = snapshot["quota_handlers"]["scheduler"] | ||
| assert scheduler["period"] == 5 | ||
| assert scheduler["database_reconnection_count"] == 10 | ||
| assert scheduler["database_reconnection_delay"] == 2 | ||
|
|
||
| def test_quota_handlers_none(self) -> None: | ||
| """Test quota_handlers when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["quota_handlers"]["sqlite"]["db_path"] == NOT_CONFIGURED | ||
| assert snapshot["quota_handlers"]["postgres"]["host"] == NOT_CONFIGURED | ||
|
|
||
| def test_byok_rag_extraction(self) -> None: | ||
| """Test byok_rag list extraction with masking.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| byok = snapshot["byok_rag"] | ||
| assert isinstance(byok, list) | ||
| assert len(byok) == 1 | ||
| assert byok[0]["rag_id"] == "my-rag" | ||
| assert byok[0]["rag_type"] == "inline::faiss" | ||
| assert byok[0]["embedding_model"] == "all-MiniLM-L6-v2" | ||
| assert byok[0]["embedding_dimension"] == 384 | ||
| assert byok[0]["vector_db_id"] == "my-vector-db" | ||
| assert byok[0]["db_path"] == CONFIGURED | ||
| assert byok[0]["score_multiplier"] == 1.5 | ||
| assert byok[0]["host"] == CONFIGURED | ||
| assert byok[0]["port"] == CONFIGURED | ||
| assert byok[0]["db"] == CONFIGURED | ||
| assert byok[0]["user"] == CONFIGURED | ||
| assert byok[0]["password"] == CONFIGURED | ||
|
|
||
| def test_byok_rag_empty(self) -> None: | ||
| """Test byok_rag when empty.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["byok_rag"] == [] | ||
|
|
||
| def test_a2a_state_fields(self) -> None: | ||
| """Test a2a_state fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| a2a = snapshot["a2a_state"] | ||
| assert a2a["sqlite"]["db_path"] == CONFIGURED | ||
| assert a2a["postgres"]["host"] == CONFIGURED | ||
| assert a2a["postgres"]["port"] == 5432 | ||
| assert a2a["postgres"]["db"] == CONFIGURED | ||
| assert a2a["postgres"]["user"] == CONFIGURED | ||
| assert a2a["postgres"]["password"] == CONFIGURED | ||
| assert a2a["postgres"]["namespace"] == CONFIGURED | ||
| assert a2a["postgres"]["ssl_mode"] == "verify-full" | ||
| assert a2a["postgres"]["gss_encmode"] == "prefer" | ||
| assert a2a["postgres"]["ca_cert_path"] == CONFIGURED | ||
|
|
||
| def test_a2a_state_none(self) -> None: | ||
| """Test a2a_state when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["a2a_state"]["sqlite"]["db_path"] == NOT_CONFIGURED | ||
| assert snapshot["a2a_state"]["postgres"]["host"] == NOT_CONFIGURED | ||
|
|
||
| def test_a2a_agents_extraction(self) -> None: | ||
| """Test a2a_agents list extraction with masking.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| agents = snapshot["a2a_agents"]["agents"] | ||
| assert isinstance(agents, list) | ||
| assert len(agents) == 1 | ||
| assert agents[0]["name"] == "external-agent" | ||
| assert agents[0]["url"] == CONFIGURED | ||
| assert agents[0]["auth_token"] == CONFIGURED | ||
| assert agents[0]["timeout"] == 30 | ||
| assert agents[0]["max_retries"] == 3 | ||
|
|
||
| def test_a2a_agents_none(self) -> None: | ||
| """Test a2a_agents when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["a2a_agents"]["agents"] == NOT_CONFIGURED | ||
|
|
||
| def test_splunk_fields(self) -> None: | ||
| """Test splunk fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| splunk = snapshot["splunk"] | ||
| assert splunk["enabled"] is True | ||
| assert splunk["url"] == CONFIGURED | ||
| assert splunk["token_path"] == CONFIGURED | ||
| assert splunk["index"] == CONFIGURED | ||
| assert splunk["source"] == "lightspeed-stack" | ||
| assert splunk["timeout"] == 5 | ||
| assert splunk["verify_ssl"] is True | ||
|
|
||
| def test_splunk_none(self) -> None: | ||
| """Test splunk when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["splunk"]["enabled"] is None | ||
| assert snapshot["splunk"]["url"] == NOT_CONFIGURED | ||
| assert snapshot["splunk"]["token_path"] == NOT_CONFIGURED | ||
| assert snapshot["splunk"]["index"] == NOT_CONFIGURED | ||
| assert snapshot["splunk"]["source"] is None | ||
|
|
||
| def test_rag_fields(self) -> None: | ||
| """Test rag strategy fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["rag"]["inline"] == ["okp", "my-rag"] | ||
| assert snapshot["rag"]["tool"] == ["my-rag"] | ||
|
|
||
| def test_rag_defaults(self) -> None: | ||
| """Test rag strategy fields with defaults.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["rag"]["inline"] == [] | ||
| assert snapshot["rag"]["tool"] == [] | ||
|
|
||
| def test_okp_fields(self) -> None: | ||
| """Test okp fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["okp"]["rhokp_url"] == CONFIGURED | ||
| assert snapshot["okp"]["offline"] is True | ||
| assert snapshot["okp"]["chunk_filter_query"] == CONFIGURED | ||
|
|
||
| def test_okp_defaults(self) -> None: | ||
| """Test okp fields with defaults.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["okp"]["rhokp_url"] == NOT_CONFIGURED | ||
| assert snapshot["okp"]["offline"] is True | ||
| assert snapshot["okp"]["chunk_filter_query"] == NOT_CONFIGURED | ||
|
|
||
| def test_reranker_fields(self) -> None: | ||
| """Test reranker fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["reranker"]["enabled"] is True | ||
| assert snapshot["reranker"]["model"] == "cross-encoder/ms-marco-MiniLM-L6-v2" | ||
|
|
||
| def test_reranker_defaults(self) -> None: | ||
| """Test reranker fields with defaults.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["reranker"]["enabled"] is False | ||
| assert snapshot["reranker"]["model"] == "cross-encoder/ms-marco-MiniLM-L6-v2" | ||
|
|
||
| def test_approvals_fields(self) -> None: | ||
| """Test approvals fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["approvals"]["approval_timeout_seconds"] == 600 | ||
| assert snapshot["approvals"]["approval_retention_days"] == 90 | ||
|
|
||
| def test_approvals_defaults(self) -> None: | ||
| """Test approvals fields with defaults.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["approvals"]["approval_timeout_seconds"] == 300 | ||
| assert snapshot["approvals"]["approval_retention_days"] == 30 | ||
|
|
||
| def test_rlsapi_v1_fields(self) -> None: | ||
| """Test rlsapi_v1 fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["rlsapi_v1"]["allow_verbose_infer"] is True | ||
| assert snapshot["rlsapi_v1"]["quota_subject"] == "user_id" | ||
|
|
||
| def test_rlsapi_v1_defaults(self) -> None: | ||
| """Test rlsapi_v1 fields with defaults.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["rlsapi_v1"]["allow_verbose_infer"] is False | ||
| assert snapshot["rlsapi_v1"]["quota_subject"] is None | ||
|
|
||
| def test_saved_prompts_fields(self) -> None: | ||
| """Test saved_prompts fields extraction.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["saved_prompts"]["max_prompts_per_user"] == 100 | ||
| assert snapshot["saved_prompts"]["max_display_name_length"] == 200 | ||
| assert snapshot["saved_prompts"]["max_content_length"] == 5000 | ||
|
|
||
| def test_saved_prompts_defaults(self) -> None: | ||
| """Test saved_prompts fields with defaults.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["saved_prompts"]["max_prompts_per_user"] == 50 | ||
| assert snapshot["saved_prompts"]["max_display_name_length"] == 255 | ||
| assert snapshot["saved_prompts"]["max_content_length"] == 10000 | ||
|
|
||
| def test_skills_paths_masked(self) -> None: | ||
| """Test skills paths is masked as sensitive.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["skills"]["paths"] == CONFIGURED | ||
|
|
||
| def test_skills_none(self) -> None: | ||
| """Test skills when not configured.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["skills"]["paths"] == NOT_CONFIGURED | ||
|
|
||
| def test_deployment_environment_passthrough(self) -> None: | ||
| """Test deployment_environment passes through.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_fully_populated_config()) | ||
| assert snapshot["deployment_environment"] == "production" | ||
|
|
||
| def test_deployment_environment_default(self) -> None: | ||
| """Test deployment_environment with default value.""" | ||
| snapshot = build_lightspeed_stack_snapshot(build_minimal_config()) | ||
| assert snapshot["deployment_environment"] == "development" | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
This block largely duplicates tests/unit/telemetry/test_configuration_snapshot_new_sections.py.
Nearly every method added here has a same-name, same-assertion twin in the new module — test_llama_stack_timeout_passthrough, test_llama_stack_config_profile_masked, test_inference_max_tool_calls_passthrough, test_customization_agent_card_path_masked, test_skills_paths_masked, test_deployment_environment_passthrough, plus the azure / splunk / okp / reranker / approvals / rlsapi_v1 / saved_prompts / quota / byok / a2a / cache / compaction groups. Every future registry change then needs two edits, and the duplication is what forced the module-level too-many-public-methods disable at Lines 3-4.
Suggest keeping the "fully-populated vs minimal" section coverage in the new module (it already has full_snapshot / minimal_snapshot fixtures) and trimming this class to the unique cases — i.e. the *_none / default-value variants the new module lacks. Secondary point: these methods call build_fully_populated_config() per test (~60 rebuilds) instead of reusing a fixture.
🤖 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/unit/telemetry/test_configuration_snapshot.py` around lines 499 - 956,
Remove the duplicated fully-populated assertions from the existing snapshot test
class, retaining only unique *_none and default-value coverage that is absent
from test_configuration_snapshot_new_sections.py. Reuse that module’s
full_snapshot and minimal_snapshot fixtures for retained tests instead of
rebuilding configurations with build_fully_populated_config() or
build_minimal_config() in each method, and remove the now-unnecessary
too-many-public-methods suppression if applicable.
are-ces
marked this pull request as draft
July 29, 2026 09:22
…pshot Add ~120 missing configuration fields across 20 sections to the LIGHTSPEED_STACK_FIELDS allowlist in the telemetry snapshot module, properly classifying each as PASSTHROUGH or SENSITIVE. New sections covered: - Service (base_url, root_path) - Llama Stack (timeout, max_retries, retry_delay, allow_degraded_mode, config) - Inference (context_windows, max_infer_iters, max_tool_calls, providers) - Authentication (skip_for_metrics, api_key_config, rh_identity_config, trusted_proxy_config) - Azure Entra ID (tenant_id, client_id, client_secret, scope) - Customization (profile_path, disable_shield_ids_override, agent_card_path) - Conversation Cache (type, memory, sqlite, postgres) - Conversation Compaction (enabled, threshold_ratio, token_floor, buffer_turns, buffer_max_ratio) - Quota Handlers (sqlite, postgres, limiters, scheduler, enable_token_history) - BYOK RAG (rag_id, rag_type, embedding_model, db_path, host, port, etc.) - A2A State (sqlite, postgres) - A2A Agents (name, url, auth_token, timeout, max_retries) - Splunk (enabled, url, token_path, index, source, timeout, verify_ssl) - RAG Strategy (inline, tool) - OKP (rhokp_url, offline, chunk_filter_query) - Reranker (enabled, model) - Approvals (approval_timeout_seconds, approval_retention_days) - rlsapi v1 (allow_verbose_infer, quota_subject) - Saved Prompts (max_prompts_per_user, max_display_name_length, max_content_length) - Skills (paths) - Deployment Environment - MCP Servers (authorization_headers, headers, require_approval, timeout) Also fixes mask_value to treat empty strings as not_configured for SENSITIVE fields, matching the behavior for None values. Spec: LCORE-2923 Harness: Three-Layer Defense verified
are-ces
force-pushed
the
feat/lcore-2923-telemetry-snapshot-fields
branch
from
July 29, 2026 09:35
84bcccc to
302af5a
Compare
are-ces
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add ~120 missing configuration fields across 20 sections to the telemetry snapshot module's
LIGHTSPEED_STACK_FIELDSallowlist, properly classifying each asPASSTHROUGHorSENSITIVE. Also fixesmask_valueto treat empty strings asnot_configuredforSENSITIVEfields.Spec
LCORE-2923
Changes
src/telemetry/configuration_snapshot.pyLIGHTSPEED_STACK_FIELDSacross 20 sections; fixedmask_valueto treat empty strings asnot_configuredtests/unit/telemetry/conftest.pybuild_fully_populated_config/build_minimal_configfixtures for all new sectionstests/unit/telemetry/test_configuration_snapshot.pytests/unit/telemetry/test_configuration_snapshot_new_sections.pyThree-Layer Defense Results
New Sections Covered
base_url,root_pathtimeout,max_retries,retry_delay,allow_degraded_mode,config.*context_windows,max_infer_iters,max_tool_calls,providers[]skip_for_metrics,api_key_config.*,rh_identity_config.*,trusted_proxy_config.*tenant_id,client_id,client_secret,scopeprofile_path,disable_shield_ids_override,agent_card_pathtype,memory.*,sqlite.*,postgres.*enabled,threshold_ratio,token_floor,buffer_turns,buffer_max_ratiosqlite.*,postgres.*,limiters[],scheduler.*,enable_token_historyrag_id,rag_type,embedding_model,db_path,host,port, etc.sqlite.*,postgres.*name,url,auth_token,timeout,max_retriesenabled,url,token_path,index,source,timeout,verify_sslinline,toolrhokp_url,offline,chunk_filter_queryenabled,modelapproval_timeout_seconds,approval_retention_daysallow_verbose_infer,quota_subjectmax_prompts_per_user,max_display_name_length,max_content_lengthpathsauthorization_headers,headers,require_approval,timeoutBug Fix
mask_value()now treats empty strings ("") asnot_configuredforSENSITIVEfields, matching the existing behavior forNonevalues. Previously, an empty string would incorrectly return"configured".Testing
uv run make test-unitNotes
"configured"/"not_configured"Summary by CodeRabbit
New Features
Documentation
Tests