Skip to content

feat(desktop): accept K/M context window inputs - #5171

Merged
me2seeks merged 1 commit into
apache:mainfrom
sunrioa:feat/5168-context-window-units
Sep 12, 2026
Merged

feat(desktop): accept K/M context window inputs#5171
me2seeks merged 1 commit into
apache:mainfrom
sunrioa:feat/5168-context-window-units

Conversation

@sunrioa

@sunrioa sunrioa commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Allow context window inputs in Desktop model settings to accept decimal K/M shorthand, such as 128K, 1M, and 1.5m, alongside plain integers.

Both adding and editing models use the same parser. Values remain positive safe integers in storage (K = 1,000; M = 1,000,000). Invalid input shows a validation error and cannot be saved. Single-click saving, cancellation, and clearing an existing declaration are preserved.

Fixes #5168

Verification

  • Passed root build, lint, format:check, typecheck, and Desktop/UI Knip.
  • Passed all 2,484 Desktop tests and 37 Desktop E2E tests.
  • Passed Storybook build and render smoke: 332 stories, 359 theme renders.
  • Passed renderer architecture, Astryx inventory and inventory tests, theme drift, E2E budget, and ASF header checks.

Before: entering 1M leaves the field empty and triggers the required-field error.

Before: required-field error

After: adding a model with 1M.

After: adding a model with 1M

After: editing a model's context window to 1.5m.

After: editing the context window to 1.5m

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex — implementation, regression tests, and validation; reviewed by the author.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 10, 2026

@me2seeks me2seeks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PR 5171 Review

结论

APPROVE
K/M 上下文窗口输入的解析器 parseContextWindowInputcontext-window-input.ts)逻辑核实:用字符串移位而非浮点乘法避免 1.001*1000 精度问题,fraction.slice(places) 含非零即拒绝小数残留(1.0000001M→null),Number.isSafeInteger && >0 兜底;负数、1e61MB.5M1. 均被正则拒绝。两处 UI(add-model 对话框、connection-detail 编辑行)改为同一 parser;编辑行用 changeContextWindow 把合法值/空串写入 draft,非法文本保留显示但不覆盖既有声明(provider-connection-detail.ts:313-318),且 canSave/onSave 双重挡住非法保存。测试(含 E2E context-window-save.spec.ts 往返 1M/1.5m/256K/清空)覆盖正反例。

@me2seeks me2seeks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maka auto review

Reviewed commit 293a308. No actionable findings. Both model forms share an exact decimal K/M parser and persist positive safe integer token counts. Invalid-input blocking, cancellation, and clearing declarations remain coherent.

Validation: full Desktop build with workspace dependencies passed; 20 focused parser/profile-draft tests passed; the real Electron context-window-save E2E passed on Linux with isolated fixture data, covering one-click focused saving, 1M/1.5m persistence, invalid input, cancellation, and clearing. No Windows/macOS packaged-app validation was performed in this review.

中文

已复核当前提交,未发现需要修改的问题。添加和编辑模型共用精确的十进制 K/M 解析器,保存值仍为正安全整数;非法输入、取消和清空声明处理一致。

完整 Desktop 及依赖构建成功,20 项相关单元测试通过,Linux 真实 Electron 窗口测试通过,覆盖聚焦时一次点击保存、1M/1.5m 持久化、非法输入、取消和清空。使用隔离测试数据,本轮未进行 Windows/macOS 打包应用验证。

@me2seeks
me2seeks merged commit 05164f5 into apache:main Sep 12, 2026
2 checks passed
@sunrioa
sunrioa deleted the feat/5168-context-window-units branch September 12, 2026 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(desktop): accept K/M suffixes in model context-window inputs

2 participants