Skip to content

perf(intelligent-assistant): make GET /v1/models non-blocking - #4839

Open
JslYoon wants to merge 6 commits into
redhat-developer:mainfrom
JslYoon:perf-models-vision-probe-background
Open

JslYoon wants to merge 6 commits into
redhat-developer:mainfrom
JslYoon:perf-models-vision-probe-background

Conversation

@JslYoon

@JslYoon JslYoon commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

GET /v1/models probed each LLM's vision capability inline, awaiting up to VISION_PROBE_TIMEOUT_MS (10s) per model. On a cold cache this delayed the response by up to ~10s.

This change serves vision support from the capabilities cache and moves probing off the request path:

  • Warm at startup — on router load, the model list is fetched and every un-cached LLM is probed in the background, so the first real request is already enriched.
  • Non-blocking on miss — a request never awaits a probe. A cache hit returns the real value; a miss returns the conservative default (supportsVision: false) immediately and triggers a single background probe (deduped per model via an in-flight set).
  • Self-healing — because only cache misses are probed, the cache re-probes models that are new to the LCS list or whose previous probe failed and was not cached, while leaving known models untouched.

The per-user POST /v1/validate-model-vision path is unchanged (it still awaits, since it needs the answer).

Behavior change

On a cold cache, an LLM may report supportsVision: false for the brief window between the first request and the background probe completing; the next request reports the real value.

Testing

  • Updated the supportsVision enrichment suite for the non-blocking + warm-on-boot contract; added coverage for startup warm-up and lazy self-heal when the LCS list changes.
  • yarn backstage-cli package test — 335/335 pass.
  • yarn tsc:full — clean.
  • Verified live against a running backend + Lightspeed service: /v1/models returns in ~50ms (was up to 10s), fully enriched.

🤖 Generated with Claude Code

@rhdh-gh-app

rhdh-gh-app Bot commented Sep 17, 2026

Copy link
Copy Markdown

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 patch v5.2.0

The /v1/models handler probed each LLM's vision support inline, awaiting
up to VISION_PROBE_TIMEOUT_MS per model, so a cold cache could delay the
response by ~10s. Serve vision support from the capabilities cache
instead: warm the cache at router startup and probe cache misses in the
background (deduped per model), returning the model list immediately. The
cache self-heals on a miss — a model new to the LCS list, or one whose
previous probe failed and was not cached.

Signed-off-by: Lucas <lyoon@redhat.com>
Assisted-by: Claude Opus 4.8
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@JslYoon
JslYoon force-pushed the perf-models-vision-probe-background branch from 2a148c1 to c737658 Compare September 17, 2026 03:39
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.12%. Comparing base (b194796) to head (ddce2b0).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4839      +/-   ##
==========================================
- Coverage   63.81%   60.12%   -3.69%     
==========================================
  Files        2698     2672      -26     
  Lines      107951   106032    -1919     
  Branches    30234    29773     -461     
==========================================
- Hits        68888    63757    -5131     
- Misses      38541    41765    +3224     
+ Partials      522      510      -12     
Flag Coverage Δ *Carryforward flag
adoption-insights 84.77% <ø> (ø) Carriedforward from d496a67
ai-integrations 79.78% <ø> (-4.56%) ⬇️ Carriedforward from d496a67
app-defaults 55.69% <ø> (-7.95%) ⬇️ Carriedforward from d496a67
augment 46.67% <ø> (ø) Carriedforward from d496a67
boost 84.97% <ø> (ø) Carriedforward from d496a67
bulk-import 73.12% <ø> (ø) Carriedforward from d496a67
cost-management 13.53% <ø> (ø) Carriedforward from d496a67
dcm 73.47% <ø> (ø) Carriedforward from d496a67
e2e-adoption-insights 60.00% <ø> (ø) Carriedforward from d496a67
e2e-extensions 62.31% <ø> (ø) Carriedforward from d496a67
e2e-global-header 51.82% <ø> (ø) Carriedforward from d496a67
e2e-homepage 61.11% <ø> (ø) Carriedforward from d496a67
e2e-intelligent-assistant 46.01% <ø> (ø) Carriedforward from d496a67
e2e-orchestrator 49.49% <ø> (ø) Carriedforward from d496a67
e2e-orchestrator-plugin 49.48% <ø> (ø) Carriedforward from d496a67
e2e-quickstart 55.21% <ø> (ø) Carriedforward from d496a67
e2e-scorecard 49.83% <ø> (ø) Carriedforward from d496a67
e2e-theme 16.36% <ø> (ø) Carriedforward from d496a67
extensions 58.30% <ø> (ø) Carriedforward from d496a67
global-floating-action-button 71.18% <ø> (ø) Carriedforward from d496a67
global-header 67.76% <ø> (ø) Carriedforward from d496a67
homepage 48.49% <ø> (-6.68%) ⬇️ Carriedforward from d496a67
install-dynamic-plugins 71.04% <ø> (-2.49%) ⬇️ Carriedforward from d496a67
intelligent-assistant 78.07% <94.44%> (+0.03%) ⬆️
konflux 91.98% <ø> (ø) Carriedforward from d496a67
lightspeed 69.02% <ø> (ø) Carriedforward from d496a67
mcp-integrations 84.46% <ø> (ø) Carriedforward from d496a67
orchestrator 77.32% <ø> (-0.37%) ⬇️ Carriedforward from d496a67
quickstart 63.74% <ø> (ø) Carriedforward from d496a67
sandbox 79.56% <ø> (ø) Carriedforward from d496a67
scorecard 88.37% <ø> (-0.54%) ⬇️ Carriedforward from d496a67
theme 87.91% <ø> (-0.03%) ⬇️ Carriedforward from d496a67
translations 5.12% <ø> (ø) Carriedforward from d496a67
x2a 13.86% <ø> (-64.58%) ⬇️ Carriedforward from d496a67

*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 b194796...ddce2b0. 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.

@JslYoon

JslYoon commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 18, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:37 PM UTC · Completed 3:52 PM UTC

Commit: 83cf99c · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.05

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Sep 18, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Small, well-tested performance refactor (3 files, 253 lines, 0.33 test ratio) with no security or dependency concerns, but the core router files show moderate multi-author contention and a handful of recent fix commits, yielding a moderate composite risk.

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

High

  • [debug code left in production] workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts:755 — Two console.log statements (lines 755 and 766) were left in the production request handler for GET /v1/models. The entire router (20+ call sites) uses the injected logger service for structured logging; console.log bypasses log-level filtering, structured metadata, and any log aggregation the deployer has configured. Additionally, the messages are misleading: 'vision probe success' fires on a cache hit (even when the cached value is false), and 'vision probe failed' fires on a cache miss (not an actual probe failure).
    Remediation: Remove both console.log calls. If per-model cache-hit/miss logging is desired, use logger.debug(...) with accurate messages (e.g. 'cache hit' / 'cache miss').

Low

  • [truncated comment] workspaces/intelligent-assistant/plugins/intelligent-assistant-backend/src/service/router.ts:751 — The comment on line 751 reads // \identifier` is already the `provider/model` key LCS and — the sentence ends mid-clause. The base branch had a two-line comment; the second line was accidentally dropped during the refactor. Remediation: Restore the second line (// /v1/validate-model-vision use — do not re-prefix with provider_id.`) or rewrite as a single complete sentence.

Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

@sonarqubecloud

Copy link
Copy Markdown

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.

1 participant