fix: resolve all outstanding lint, type, and viewer extra issues - #39
Merged
Conversation
Gated by LYRASHIELD_PROMPT_CACHE_EXPLICIT=1. Default remains implicit. Smoke test on azure_ai/gpt-5.6-luna failed with response.incomplete/content_filter, so this stays opt-in and unmerged for now.
…r explicit prompt caching - Create .env with Azure AI credentials for local CLI usage - Auto-load .env via python-dotenv in both lyrashield and strix entry points - Add field_validator to LlmSettings: empty LLM_API_* env values -> None - Clean stale empty LLM_API_* env vars in prepare_environment() so they don't shadow AZURE_AI_* aliases via pydantic AliasChoices priority - Add ModelBehaviorError catch in run_scan: if content_filter is returned when explicit caching is enabled, fall back to implicit caching and retry - Import _prompt_cache_explicit_enabled and ModelBehaviorError in runner Verified: baseline SAFE scan (implicit) completed with 84.2% cache hit ratio. Explicit SAFE scan (LYRASHIELD_PROMPT_CACHE_EXPLICIT=1) completed without content_filter errors on azure_ai/gpt-5.6-terra.
ecryptoguru
changed the title
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR adds optional dotenv loading and empty-value cleanup, introduces opt-in explicit prompt caching for GPT-5.6 with content-filter fallback, and adds regression coverage for optional viewer dependencies and release synchronization. ChangesRuntime configuration
Explicit prompt-cache flow
Viewer extra validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Runner
participant make_model_settings
participant RootAgent
participant run_agent_loop
Runner->>make_model_settings: configure explicit options for resolved model
Runner->>RootAgent: create root agent
Runner->>run_agent_loop: execute with explicit prompt caching
run_agent_loop-->>Runner: content_filter ModelBehaviorError
Runner->>make_model_settings: disable explicit prompt-cache options
Runner->>RootAgent: recreate root agent
Runner->>run_agent_loop: retry with fallback caching
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
strix/core/runner.py (1)
469-530: 📐 Maintainability & Code Quality | 🔵 TrivialConsider adding regression coverage for the content_filter fallback path.
This retry logic (rebuild agent/settings, drop explicit caching, resend) is exactly the kind of subtle control flow that's easy to silently break; no test for it was included in the reviewed files. A test mocking
run_agent_loopto raiseModelBehaviorError("... content_filter ...")once, then asserting the retry usesprompt_cache_options=Noneand the correctinitial_input, would have caught the issues flagged above.🤖 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 `@strix/core/runner.py` around lines 469 - 530, The content_filter fallback in the root agent execution path lacks regression coverage. Add a focused test around the runner flow that mocks run_agent_loop to raise ModelBehaviorError containing “content_filter” once, then succeed, and assert the retry rebuilds model settings with prompt_cache_options=None and sends the correct rebuilt initial_input.
🤖 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 `@strix/config/settings.py`:
- Around line 107-111: The LlmSettings alias selection currently lets blank
earlier aliases shadow later configured aliases. Update _empty_env_to_none and
the settings-loading flow in strix/config/settings.py (107-111) and
lyrashield_adapter/cli.py (52-63) so every LlmSettings alias is normalized or
removed before AliasChoices selection, allowing non-empty aliases such as
OPENAI_API_KEY and AZURE_OPENAI_ENDPOINT to be selected.
In `@strix/core/runner.py`:
- Line 494: Update the fallback assignment to initial_input near build_root_task
so it respects the existing is_resume behavior: retain the empty initial input
for resumed scans and only build the root task for fresh scans. Ensure
content_filter fallback does not re-inject a root task into an already-populated
resumed session.
- Around line 484-493: Extend the error handling around run_agent_loop to
recognize Azure prompt-side openai.BadRequestError instances whose code is
"content_filter" when explicit prompt caching is enabled via
_prompt_cache_explicit_enabled or force_prompt_cache. Route them through the
same warning, implicit-cache fallback, and retry path currently used by the
ModelBehaviorError branch, while preserving normal propagation for unrelated
BadRequestError failures.
- Around line 495-530: Before the retry call to run_agent_loop, rebuild
run_config after setting its model_settings to disabled/None so it no longer
retains the original explicit prompt_cache_options. Ensure the recreated
RunConfig is the object passed to the retry, while preserving the existing retry
agent and other run configuration values.
In `@strix/interface/main.py`:
- Around line 1096-1100: Change the dotenv loading calls in
strix/interface/main.py (lines 1096-1100) and lyrashield_adapter/cli.py (lines
104-105) to use override=False or omit the override argument, preserving
explicit shell environment variables over values from .env.
- Around line 1097-1102: Restrict the exception handling around the dotenv setup
in main() to the expected missing-dependency or optional-file cases, rather than
catching Exception. Let unexpected import or load_dotenv failures propagate,
while preserving continuation when the dotenv dependency or .env file is
legitimately unavailable.
In `@tests/test_fenced_code.py`:
- Around line 16-18: Update the _viewer_extra_available dependency gate in
tests/test_fenced_code.py to require both pypdf and reportlab, matching the
module-import requirements of styix.viewer.report_pdf. Keep the existing skip
behavior, but mark the viewer extra unavailable whenever either dependency is
missing.
In `@tests/test_viewer_extra.py`:
- Around line 33-50: The dependency assertions in
test_viewer_extra_is_optional_and_includes_pdf_packages use incomplete string
parsing. Import and use a PEP 508 requirement parser to extract normalized
dependency names from both deps and extras["viewer"], then perform the pypdf,
reportlab, and viewer membership checks against parsed names while preserving
the existing cryptography version-cap assertion.
- Around line 23-26: Separate the pytest coverage for tests/test_viewer_extra.py
into explicit base-only and viewer-extra CI matrix jobs, using isolated UV
environments and locks so the base job excludes pypdf/reportlab while the viewer
job includes only the viewer extras. Ensure
test_base_install_does_not_import_pdf_packages runs in the base environment and
test_viewer_extra_imports_both_pdf_packages runs in the viewer environment,
without allowing either test to be skipped because the other dependency set is
installed.
---
Nitpick comments:
In `@strix/core/runner.py`:
- Around line 469-530: The content_filter fallback in the root agent execution
path lacks regression coverage. Add a focused test around the runner flow that
mocks run_agent_loop to raise ModelBehaviorError containing “content_filter”
once, then succeed, and assert the retry rebuilds model settings with
prompt_cache_options=None and sends the correct rebuilt initial_input.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 387995bb-00c6-4e14-9da5-aef4203d5723
📒 Files selected for processing (8)
lyrashield_adapter/cli.pystrix/config/settings.pystrix/core/inputs.pystrix/core/runner.pystrix/interface/main.pytests/test_fenced_code.pytests/test_inputs.pytests/test_viewer_extra.py
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 `@tests/test_viewer.py`:
- Around line 488-491: Update the test setup around serve() so the fake
report_pdf module assignment occurs inside the try block, ensuring cleanup runs
when server startup raises. Initialize httpd before the try and guard its
shutdown in finally so shutdown occurs only after successful creation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ecryptoguru
force-pushed
the
fix/viewer-extra
branch
from
July 27, 2026 19:16
b47bcdd to
9ca6f09
Compare
ecryptoguru
added a commit
that referenced
this pull request
Jul 30, 2026
* docs: add PR #22-#40 entries to UPGRADES.md ledger Updates the ownership and upstream-import ledger with entries for engine PRs #22 (Sol retirement, token caps), #26 (upstream sync through 8157ccb), #33 (Azure GPT-5.6 hardening), #35 (cost/cache optimization), #36 (synchronous session close), #39 (lint/type/viewer fixes), and #40 (run.json progress fields). * fix(docs): remove incorrect PR #36 cross-reference from cache entry Address CodeRabbit review on PR #41 — the parenthetical '(PR #36 follow-up)' misattributed cache work to the synchronous session-closing PR.
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.
Fixes the outstanding lint/type issues and completes the viewer extra plumbing described in #27.
Changes:
Verification: