Skip to content

agent: read TOML literal-quoted Kimi model keys - #446

Merged
yetone merged 2 commits into
yetone:mainfrom
charan-rathore:fix-kimi-toml-literal-keys
Oct 1, 2026
Merged

yetone merged 2 commits into
yetone:mainfrom
charan-rathore:fix-kimi-toml-literal-keys

Conversation

@charan-rathore

Copy link
Copy Markdown
Contributor

kimiOwnOptions (internal/agent/kimi.go) unquotes model table keys with strconv.Unquote, which only handles double-quoted strings. A model keyed with a TOML literal (single-quoted) string, [models.'kimi k2.5'], keeps its quotes, fails Unquote, and is then silently skipped by the dotted-key branch because the quoted name contains a dot. The single-quoted form is required for keys containing spaces and is what TOML writers emit for such keys, so a documented config shape loses the model from the picker.

When Unquote fails, strip surrounding single quotes directly; a TOML literal string has no escapes, so the strip is exact.

@yetone

yetone commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Thanks! The picker now lists [models.'kimi k2.5'] — confirmed the test fails before and passes after. Two things: (1) [models.'a'.'b'] now yields a bogus option a'.'b (main skipped it) — please reject the stripped name if it still contains '; (2) ownModel in kimi.go looks the key up as models."<key>", so after switching to a magpie model and back to default, a literal-keyed (or bare-keyed) model isn't restored and default_model ends up empty. Could you match tables by their decoded key in both places (one small helper), with a test that sets a magpie model and resets back to a '…'-keyed default?

@charan-rathore

Copy link
Copy Markdown
Contributor Author

Thanks, both fixed in 4e4d38c.

  1. [models.'a'.'b'] is rejected now: a stripped name that still contains ' is not offered as an option.
  2. The picker, restore and default check now share one helper that matches tables by their decoded key, so a '…'-keyed or bare-keyed default is restored after switching to a magpie model and back. Added a test that sets a magpie model and resets to a literal-keyed default, plus a nested-header case and a bare-key case.

internal/agent and internal/edit tests pass.

@yetone

yetone commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Thanks for the quick follow-up — re-reviewed at 4e4d38c and this looks good to merge.

  • [models.'a'.'b'] is no longer offered (TestKimiNestedLiteralKeyIsNotAModel fails on dee2356, passes now).
  • Restore goes through kimiModelTableForKey, so after magpie kimi magpie/<p>/<m> and magpie kimi default, a 'kimi k2.5'-keyed default comes back. In a sandbox HOME main drops default_model entirely; this branch restores "kimi k2.5". TestKimiRestoresDecodedModelKey (literal and bare) and TestKimiLiteralQuotedModelKey fail without the kimi.go change.
  • Other header forms still behave: basic-quoted with escapes, bare, "dotted.name", 'lit.dot' and [ models . 'spaced key' ] are listed and resolve to their own tables; 'a'.b, "a".'b', kimi.sub and 'a'.'b'.'c' are skipped.

Merged onto current main cleanly; vet, the darwin/linux/windows builds and go test -tags nogui ./... all pass.

@yetone
yetone merged commit d21f2f6 into yetone:main Oct 1, 2026
7 checks passed
@charan-rathore
charan-rathore deleted the fix-kimi-toml-literal-keys branch October 1, 2026 19:19
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