Skip to content

(fix) deep context intelligent assistant model vision verification no cache - #4147

Merged
JslYoon merged 4 commits into
redhat-developer:mainfrom
JslYoon:main
Aug 4, 2026
Merged

(fix) deep context intelligent assistant model vision verification no cache#4147
JslYoon merged 4 commits into
redhat-developer:mainfrom
JslYoon:main

Conversation

@JslYoon

@JslYoon JslYoon commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes deep context model vision verification to not cache false results when the upstream returns a 5xx error.

Why

Previously, when the vision verification endpoint received a 5xx from the upstream model server (e.g., temporary outage, overloaded), the result was cached as supportsVision: false. This meant the model was permanently marked as non-vision-capable until cache expiry, even though the failure was transient. Users would lose deep context (image attachment) functionality for that model until the cache cleared.

What changed

  • Removed the else if (status >= 500) and else branches from the vision verification handler in router.ts — the endpoint now only caches and responds for successful (vision-confirmed) results. All other outcomes (4xx, 5xx, network errors) fall through to the existing catch block, which logs but does not cache.
  • Removed 3 tests from router.test.ts that validated the old caching-on-failure behavior:
    • returns false when model lacks vision
    • returns false when model is not found
    • returns 502 without caching when upstream returns 5xx

Impact

  • Vision verification failures are no longer permanently cached — retries will re-check the upstream
  • Eliminates false negatives for vision support caused by transient upstream errors

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-gh-app

rhdh-gh-app Bot commented Aug 3, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-intelligent-assistant-backend workspaces/intelligent-assistant/plugins/intelligent-assistant-backend minor v3.2.0

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Avoid caching model-vision verification failures for upstream 5xx responses

🐞 Bug fix ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add a changeset for a minor backend release documenting the cache behavior change.
• Simplify /v1/validate-model-vision handling to only cache positive (supportsVision=true) results.
• Remove explicit non-OK (4xx/5xx) response handling to prevent caching false on upstream errors.
Diagram

graph TD
  A["Client"] --> B["POST /v1/validate-model-vision"] --> C{{"LCS /v1/responses"}}
  B -- "cache true" --> D[(ModelCapabilitiesCache)]
  subgraph Legend
    direction LR
    _api["API Route"] ~~~ _ext{{"External API"}} ~~~ _cache[("Cache")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep explicit non-OK handling; only skip caching on 5xx
  • ➕ Preserves existing contract: return supportsVision=false for expected 4xx cases (model lacks vision/not found)
  • ➕ Avoids hanging requests when fetch returns non-OK without throwing
  • ➕ Still prevents poisoning the cache during transient upstream failures
  • ➖ Slightly more branching in the route handler
2. Tri-state cache (true/false/unknown) with separate TTLs
  • ➕ Avoids repeated upstream calls for known negatives while preventing long-lived caching of transient failures
  • ➕ Makes transient error behavior explicit (unknown)
  • ➖ More complexity (data model + tests) for a small endpoint
3. Cache per-status class (2xx true, 4xx false, 5xx uncached) with metrics
  • ➕ Clear policy; easier to reason about and observe via metrics/logs
  • ➕ Keeps cache useful while avoiding bad entries
  • ➖ Requires adding metrics/logging and potentially configuration knobs

Recommendation: If the goal is strictly “don’t cache false results when upstream returns 5xx”, the best approach is to restore explicit handling for non-OK responses: treat 5xx as a 502 without caching, and treat 4xx as supportsVision=false (optionally cached). The current diff removes all non-OK response handling, which risks leaving requests without a response when fetch returns 4xx/5xx (since fetch typically does not throw on non-2xx).

Files changed (2) +5 / -12

Bug fix (1) +0 / -12
router.tsAdjust validate-model-vision caching to only persist successful results +0/-12

Adjust validate-model-vision caching to only persist successful results

• Updates the /v1/validate-model-vision route to cache and return results only when the upstream vision test succeeds (OK). Removes prior branches that handled upstream 5xx with a 502 response and cached supportsVision=false for other non-OK responses.

workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts

Other (1) +5 / -0
late-snails-battle.mdAdd changeset for backend minor release (vision cache behavior) +5/-0

Add changeset for backend minor release (vision cache behavior)

• Introduces a changeset bumping the intelligent-assistant backend plugin as a minor release. Documents that model vision verification results for upstream 500s should no longer be cached.

workspaces/intelligent-assistant/.changeset/late-snails-battle.md

@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.03%. Comparing base (58144ee) to head (4ea7697).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4147      +/-   ##
==========================================
- Coverage   58.12%   58.03%   -0.09%     
==========================================
  Files        2422     2411      -11     
  Lines       96490    96287     -203     
  Branches    26854    26780      -74     
==========================================
- Hits        56081    55877     -204     
- Misses      40203    40214      +11     
+ Partials      206      196      -10     
Flag Coverage Δ *Carryforward flag
adoption-insights 84.55% <ø> (ø) Carriedforward from baee808
ai-integrations 69.61% <ø> (-0.16%) ⬇️ Carriedforward from baee808
app-defaults 69.79% <ø> (ø) Carriedforward from baee808
augment 46.67% <ø> (ø) Carriedforward from baee808
boost 76.77% <ø> (ø) Carriedforward from baee808
bulk-import 72.56% <ø> (ø) Carriedforward from baee808
cost-management 13.55% <ø> (ø) Carriedforward from baee808
dcm 60.72% <ø> (ø) Carriedforward from baee808
extensions 56.59% <ø> (ø) Carriedforward from baee808
global-floating-action-button 71.18% <ø> (ø) Carriedforward from baee808
global-header 66.50% <ø> (ø) Carriedforward from baee808
homepage 47.50% <ø> (-0.09%) ⬇️ Carriedforward from baee808
install-dynamic-plugins 59.95% <ø> (ø) Carriedforward from baee808
intelligent-assistant 74.59% <ø> (-0.03%) ⬇️
konflux 91.98% <ø> (ø) Carriedforward from baee808
lightspeed 69.02% <ø> (ø) Carriedforward from baee808
mcp-integrations 83.40% <ø> (ø) Carriedforward from baee808
orchestrator 66.87% <ø> (ø) Carriedforward from baee808
quickstart 63.67% <ø> (-0.08%) ⬇️ Carriedforward from baee808
sandbox 79.56% <ø> (ø) Carriedforward from baee808
scorecard 85.27% <ø> (-0.71%) ⬇️ Carriedforward from baee808
theme 88.52% <ø> (-0.25%) ⬇️ Carriedforward from baee808
translations 5.12% <ø> (ø) Carriedforward from baee808
x2a 79.20% <ø> (ø) Carriedforward from baee808

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58144ee...4ea7697. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… not cached for 500 error

Signed-off-by: Lucas <lyoon@redhat.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 4, 2026

Copy link
Copy Markdown

@JslYoon JslYoon changed the title (fix) deep context intteligent assistant model vision verification no cache (fix) deep context intelligent assistant model vision verification no cache Aug 4, 2026
@JslYoon
JslYoon merged commit 0f32982 into redhat-developer:main Aug 4, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants