Skip to content

fix(vision): make image analysis streaming opt-in - #20

Open
qiuYliangM wants to merge 2 commits into
Pinvou:pinvou3-cleanfrom
qiuYliangM:fix/vision-tool-robustness
Open

fix(vision): make image analysis streaming opt-in#20
qiuYliangM wants to merge 2 commits into
Pinvou:pinvou3-cleanfrom
qiuYliangM:fix/vision-tool-robustness

Conversation

@qiuYliangM

@qiuYliangM qiuYliangM commented Aug 20, 2026

Copy link
Copy Markdown

Summary

  • Preserve the existing non-streaming request shape unless [vision_model].stream = true is explicitly configured.
  • Add a bounded total request budget and an option to disable the existing transient-error retry policy for local process endpoints.
  • Reuse shared retry, Retry-After, error sanitization, and bounded error-body handling instead of introducing a second retry implementation.
  • Parse UTF-8 SSE chunks safely, cap response buffering, fall back to ordinary JSON when a compatible endpoint ignores streaming, reject empty analyses, and mark incomplete usable output as truncated.
  • Document the three compatibility controls.

Scope

This revision deliberately omits app-specific system prompts, default prompts, temperature overrides, and output-token overrides. Those policies belong in the application; the foundation change only supplies reusable transport and failure semantics.

Testing

  • cargo fmt --all -- --check
  • cargo test -p codewhale-tui vision::tools::tests -- --nocapture (19 passed)
  • Affected-crate all-target Clippy passed with the target branch existing lint categories allowed.
  • cargo test --workspace --all-features --locked (10,145 passed; one unrelated target-branch test failed: runtime_api::tests::skill_lifecycle_uninstall_removes_installed_skill. The same test failed again in isolation before reaching the uninstall step.)

The unmodified strict Clippy command is also blocked by 32 existing target-branch warnings in unrelated files. No warning points to this change.

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Preserved the default request wire shape
  • Harvested/co-authored credit uses a GitHub numeric noreply address
  • No UI behavior changed

Known risks

  • Streaming assumes the OpenAI-compatible data: event shape. Endpoints that ignore streaming and return ordinary JSON are handled explicitly.
  • Downstream app integration that constructed the former app-specific configuration fields must keep those policies in the app and update its config construction.

No-Issue: hardens the existing vision tool for slow and local OpenAI-compatible endpoints without expanding foundation policy.

@github-actions

Copy link
Copy Markdown

Thanks @qiuYliangM for taking the time to contribute.

This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

Preserve the existing non-streaming request shape by default while allowing callers to opt into SSE, a total request budget, and single-attempt behavior. Reuse the shared retry policy and HTTP error handling, reject empty analyses, bound response buffers, and report incomplete streams as truncated.

Document the compatibility controls and cover streaming, JSON fallback, timeout, UTF-8 chunking, and empty-response paths.

Co-authored-by: qiuYliangM <185303122+qiuYliangM@users.noreply.github.com>
Signed-off-by: hexin <he.xin@h3c.com>
@h3c-hexin
h3c-hexin force-pushed the fix/vision-tool-robustness branch from ea5bdf5 to 4e3b978 Compare August 26, 2026 08:33
@h3c-hexin h3c-hexin changed the title fix(vision): image_analyze 三项必要修复 + 暴露 retry_on_transient_errors fix(vision): make image analysis streaming opt-in Aug 26, 2026
Mid-stream chunk read failures and per-chunk timeouts were previously
collapsed into the same truncated flag as a clean finish_reason=length
cut, making a broken connection indistinguishable from a normal
truncation. Return execution_failed carrying the underlying cause for
mid-stream read/timeout errors (after the configured retry policy has
run), keep truncated for genuine length cuts and incomplete EOF, and
log malformed SSE frames with tracing::warn.

Document the mid-stream error semantics in CONFIGURATION.md and add
wiremock tests covering mid-stream read failure, mid-stream timeout,
and malformed-frame truncation.

Implemented with AI agent assistance (Kimi Code); reviewed and verified
with cargo test -p codewhale-tui --lib vision::tools (22 passed).

Co-authored-by: qiuYliangM <185303122+qiuYliangM@users.noreply.github.com>
Signed-off-by: luzeyang (INT) <lu.zeyang@h3c.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants