Skip to content

fix(provider): improve Kimi context, model names, and image attachments - #102

Merged
ltmoerdani merged 2 commits into
ltmoerdani:mainfrom
Wallacy:fix/open-issues-87-92-94
Aug 4, 2026
Merged

fix(provider): improve Kimi context, model names, and image attachments#102
ltmoerdani merged 2 commits into
ltmoerdani:mainfrom
Wallacy:fix/open-issues-87-92-94

Conversation

@Wallacy

@Wallacy Wallacy commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📝 What does this change?

Implements fixes for #87, #92, and #94:

  • Adds large-context options for Kimi/K3 models.
  • Adds an optional provider prefix to model names.
  • Normalizes oversized image attachments before sending them to OpenCode Go.

Closes #87
Closes #92
Closes #94

🧪 How did you test it?

  • npm test passes: 132 tests.
  • npm run compile passes.
  • npm run package successfully generates the VSIX.
  • Added automated coverage for model names, Kimi context options, and oversized image normalization.
  • No live provider request was performed in VS Code.

✅ Checklist

  • npm run compile passes
  • I tested it works
  • I updated docs/CHANGELOG if needed

@ltmoerdani

Copy link
Copy Markdown
Owner

Hey Wallacy, thanks for this. Took me a bit to go through all three fixes together.

The Kimi context tier (#87) and the provider prefix toggle (#92) look clean to me. getContextSizeOptionsForModel properly defers to models.dev tiers when they exist and only synthesizes the 256K option as a fallback, which is the right call. Extracting formatModelName into its own module is a nice cleanup too.

The image normalizer (#94) matches what opencode CLI does, so the 2000×2000 + 5MB thresholds are correct per the upstream contract. I checked the package, @silvia-odwyer/photon-node is WASM-based (~2.3MB, no native deps), which addresses the platform-specific concern I had from earlier image work. One thing I want to confirm before merging though.

There's a size guard in convertMessage() at line ~3453 that rejects top-level images above MAX_TOP_LEVEL_IMAGE_BYTES = 2_000_000 raw bytes and replaces them with a placeholder text part. Your normalizeImagePartsInPlace runs after convertMessage() builds the messages, so in the current flow an image above 2MB raw gets dropped to a placeholder before the normalizer ever sees it. That means the normalizer only fires for images that are already under 2MB raw but somehow still trip the gateway (e.g. dimensions over 2000px on a small file).

Two questions on this:

  1. Is that ordering intentional, or should normalizeImageDataUrl run before the size guard so we resize first and let the guard only catch images that are still oversized after normalization? My read of issue [BUG] Image attachments fail with HTTP 400 on OpenCode Go, while the same image+model works in opencode CLI #94 (payloadBytes=880950, a sub-1MB PNG that still 400s) suggests the dimension-based normalization is the actual fix, and the existing guard is a separate safety net. But I want to make sure I'm not missing a case where both need to fire.

  2. Related: should MAX_BASE64_BYTES (5MB) and MAX_TOP_LEVEL_IMAGE_BYTES (2MB raw) be reconciled, or are they meant to handle different layers? Right now they overlap in a slightly confusing way.

Also a minor one on candidateSizes in imageNormalizer.ts. The reduce with accumulator mutation and .at(-1) works, but it took me a couple of reads to follow the geometric decay logic. A plain while loop would read more obviously for what it's doing. Not blocking, just a readability nit.

No live provider test was run per your checklist, which is fair given the gateway behavior. I'm happy to test the normalizer path locally with a real vision model before we merge, or if you've already validated it works against the gateway, let me know and I'll take your word for it.

Tests look solid otherwise. Once we align on the guard ordering question, I'm good to merge this.

@Wallacy

Wallacy commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. You were right: the ordering was not intentional. The 2 MB raw top-level guard ran inside convertMessage() before image normalization, so it could replace images that Photon could have resized or compressed into a provider-safe payload.

I applied a follow-up that:

  • normalizes top-level images before the final payload guard;
  • removes the old 2 MB raw top-level guard;
  • uses the shared 2000×2000 / 5 MB base64 limits as the final top-level contract;
  • keeps the separate 1 MB raw guard for tool-result images, since that guard is intended to bound cumulative MCP screenshot history;
  • rewrites candidateSizes() as a plain while loop;
  • adds a regression test for a raw image above 2 MB whose base64 payload still fits the final limit.

Follow-up commit: 4572a9f

@ltmoerdani

Copy link
Copy Markdown
Owner

Yep, that nails it. I went through the diff again and the ordering is right now: normalizeImagePart fires before the guard on both paths (top-level around 3468, tool-result around 3405), and convertMessage being async doesn't break anything else since there's only the one caller at 2049.

Dropping MAX_TOP_LEVEL_IMAGE_BYTES and sharing MAX_IMAGE_BASE64_BYTES is the cleaner shape. One threshold to reason about, and the 1MB raw tool-result guard still doing its job for the MCP screenshot accumulation case from #38.

The while-loop rewrite and the 2MB-raw-but-5MB-base64 regression test, both good.

CI green on 4572a9f, mergeable is clean. I'll do a merge commit to keep both your commits in history. Thanks

@ltmoerdani
ltmoerdani merged commit a273a1f into ltmoerdani:main Aug 4, 2026
2 checks passed
ltmoerdani added a commit that referenced this pull request Aug 4, 2026
- CHANGELOG: move [Unreleased] to [0.5.0] — 2026-08-05 with section
  "Changed" noting MAX_TOP_LEVEL_IMAGE_BYTES removal in PR #102
- package.json + package-lock.json: 0.4.5 -> 0.5.0
- docs/issues/38-*: mark superseded by #94 (MAX_TOP_LEVEL_IMAGE_BYTES
  removed, replaced by image normalizer running before payload guard)
- docs/issues/47-*: rename from seq 44 to resolve collision with
  issue #94 doc, fix header title (Issue #44 -> #103)

VSIX built (opencode-copilot-chat-0.5.0.vsix, 1.82 MB, 133 tests pass)
and installed locally. Not pushed, not tagged, not published.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants