natsukium/felis
Watch
1
Code Issues 27 Pull requests 1 Projects Releases Packages Wiki Activity Actions

[v0.1/Config Review] font.size_px が TOML int を拒否し opacity/mouse の clamp が無警告 #143

New issue
Closed
opened 2026-09-05 11:57:07 +09:00 by natsukium · 2 comments
natsukium commented 2026-09-05 11:57:07 +09:00
Owner
Copy link

背景

config.toml は 1.0 で frozen label が付くが、現行の型と validation に「今しか直せない」互換性コストが残っている。

1. font.size_px = 14 が parse error になる

EffectiveConfig.font.size_px: Option<f32> は toml = "1.0" のデフォルト Deserialize を使う。TOML は int と float を区別するため size_px = 14 は invalid type: integer, expected float で DiagnosticKind::Parse Error になる。

docs と code がこれを仕様として宣言しているのが問題:

  • crates/felis-client-core/src/config.rs:500 コメント: Must be a TOML float (size_px = 14.0, not 14)
  • docs/reference/config.md Complete example: size_px = 14.0 のみ記載
  • nix/hm-module.nix の settings example も 14.0

現実に 14 と書くユーザは多く、alacritty/kitty/wezterm は int/float どちらも受ける。1.0 後に「int も受け入れる」にすると既存の error ファイルが突然 valid になるのは breaking ではないが、「一度 error だった書式が通る」より「最初から int を受け入れる」方がクリーン。1.0 で frozen 前に直すチャンス。

2. window.opacity / mouse.scroll_multiplier の clamp が無警告

validate.rs::run は font_size, fallback_chain, palette, keymap, shader のみ検証する。WindowConfig::clamped_opacity() (0.0..=1.0, NaN→1.0) と MouseConfig::clamped_scroll_multiplier() (0.1..=100.0) は validate.rs で warn せず、使用時に clamp するのみ。

結果:

  • window.opacity = 2.5 → 1.0 に clamp されるが felis config check は warn しない。reference/config.md Behavior 表は「Clamped to [0.0, 1.0] at load」と書くが実装は load 時ではなく use 時。
  • mouse.scroll_multiplier = -1 → 0.1 に反転防止 clamp されるが同様に silent。
  • window.backdrop = "blur" の OS 非対応は window stage の notice のみで config check は通る（意図通りだが、docs の表に「load vs window stage」の区別が opacity/scroll_multiplier と整合しない）。

font.size_px が Clamped を warn するのと同じ粒度で他 clamp も warn すべき。

3. theme.palette.indexed が string-key のまま

PaletteConfig.indexed: BTreeMap<String,String> は TOML の key が文字列であるための回避だが、JSON Schema では BTreeMap<String,String> のまま出力され、数値 16 が文字列 "16" として扱われる。proto の GridPaletteColor.index: uint32 と語彙がずれる。

TOML でも indexed."16" = "#..." と indexed.16 = "#..." の両方が同じ文字列 key になるため、整数キーに見えて文字列という二重性が editor 補完で紛らわしい。

問い

1.0 で以下を決める:

  1. font.size_px に int/float 両対応の custom deserializer を当て、14 と 14.0 を同値として受けるか。schemars の JsonSchema も number として両方受ける形にする。
  2. window.opacity / mouse.scroll_multiplier の範囲外を validate.rs で DiagnosticKind::Value Warning にし、felis config check で検出可能にするか。window.backdrop の OS 非対応は config check の scope 外として明記するか。
  3. theme.palette.indexed を BTreeMap<u8,String> + custom TOML deserializer (string/int 両対応) に正規化するか、現行の String キーを frozen として docs に理由を残すか。

提案

  • font.size_px は deserialize_with = "deserialize_flexible_f32" を追加し、TOML int (64) と float (f64) の両方を f32 に正規化。reference/config.md の「Must be a TOML float」記述を削除し、example に 14 と 14.0 両方が valid と追記。nix/hm-module.nix の example は 14.0 のままでも int が通ることを担保。
  • validate.rs に window_opacity / mouse_scroll_multiplier / window_backdrop の warning パスを追加し、reference/config.md Behavior 表の Stage 列を load (warn) に揃える。backdrop の OS 非対応は window stage のまま残し、表に注記。
  • theme.palette.indexed は 1.0 で String のまま凍結するなら reference/config.md と felis-config.schema.json のトップコメントに「TOML key は文字列であるため indexed は String キー」理由を frozen として記録。u8 に変えるなら BREAKING.md に base: <sha> を追加し just schema を再生成。どちらかに 1.0 で一意に決める。

判定基準

  • size_px = 14 と size_px = 14.0 が felis config check で共に valid かつ同値として show-effective に現れること。
  • window.opacity = 2 / mouse.scroll_multiplier = -1 が felis config check で Warning になること（現行の silent clamp でない）。
  • reference/config.md「Behavior on missing / malformed values」表と validate.rs の warning 対象が 1:1 で対応すること。
  • felis-config.schema.json の font.size_px が type: number で int/float 両方を受けること。

対象ファイル

  • crates/felis-client-core/src/config.rs (FontConfig.size_px, WindowConfig.opacity, MouseConfig.scroll_multiplier, PaletteConfig.indexed)
  • crates/felis-client-core/src/config/validate.rs (run, font_size に倣った新関数)
  • crates/felis-client-core/felis-config.schema.json (just schema)
  • docs/reference/config.md「Complete annotated example」「Behavior on missing / malformed values」
  • nix/hm-module.nix (settings.example)

Parent: #12 および #11 / #28

## 背景 `config.toml` は 1.0 で frozen label が付くが、現行の型と validation に「今しか直せない」互換性コストが残っている。 ### 1. `font.size_px = 14` が parse error になる `EffectiveConfig.font.size_px: Option<f32>` は `toml = "1.0"` のデフォルト Deserialize を使う。TOML は int と float を区別するため `size_px = 14` は `invalid type: integer, expected float` で `DiagnosticKind::Parse` Error になる。 docs と code がこれを仕様として宣言しているのが問題: - `crates/felis-client-core/src/config.rs:500` コメント: `Must be a TOML float (size_px = 14.0, not 14)` - `docs/reference/config.md` Complete example: `size_px = 14.0` のみ記載 - `nix/hm-module.nix` の `settings` example も `14.0` 現実に `14` と書くユーザは多く、alacritty/kitty/wezterm は int/float どちらも受ける。1.0 後に「int も受け入れる」にすると既存の error ファイルが突然 valid になるのは breaking ではないが、「一度 error だった書式が通る」より「最初から int を受け入れる」方がクリーン。1.0 で frozen 前に直すチャンス。 ### 2. `window.opacity` / `mouse.scroll_multiplier` の clamp が無警告 `validate.rs::run` は `font_size`, `fallback_chain`, `palette`, `keymap`, `shader` のみ検証する。`WindowConfig::clamped_opacity()` (`0.0..=1.0`, NaN→1.0) と `MouseConfig::clamped_scroll_multiplier()` (`0.1..=100.0`) は `validate.rs` で warn せず、使用時に clamp するのみ。 結果: - `window.opacity = 2.5` → `1.0` に clamp されるが `felis config check` は warn しない。`reference/config.md` Behavior 表は「Clamped to `[0.0, 1.0]` at load」と書くが実装は load 時ではなく use 時。 - `mouse.scroll_multiplier = -1` → `0.1` に反転防止 clamp されるが同様に silent。 - `window.backdrop = "blur"` の OS 非対応は `window` stage の notice のみで `config check` は通る（意図通りだが、docs の表に「load vs window stage」の区別が `opacity`/`scroll_multiplier` と整合しない）。 `font.size_px` が `Clamped` を warn するのと同じ粒度で他 clamp も warn すべき。 ### 3. `theme.palette.indexed` が string-key のまま `PaletteConfig.indexed: BTreeMap<String,String>` は TOML の key が文字列であるための回避だが、JSON Schema では `BTreeMap<String,String>` のまま出力され、数値 `16` が文字列 `"16"` として扱われる。proto の `GridPaletteColor.index: uint32` と語彙がずれる。 TOML でも `indexed."16" = "#..."` と `indexed.16 = "#..."` の両方が同じ文字列 key になるため、整数キーに見えて文字列という二重性が editor 補完で紛らわしい。 ## 問い 1.0 で以下を決める: 1. `font.size_px` に int/float 両対応の custom deserializer を当て、`14` と `14.0` を同値として受けるか。`schemars` の `JsonSchema` も `number` として両方受ける形にする。 2. `window.opacity` / `mouse.scroll_multiplier` の範囲外を `validate.rs` で `DiagnosticKind::Value` Warning にし、`felis config check` で検出可能にするか。`window.backdrop` の OS 非対応は `config check` の scope 外として明記するか。 3. `theme.palette.indexed` を `BTreeMap<u8,String>` + custom TOML deserializer (string/int 両対応) に正規化するか、現行の `String` キーを frozen として docs に理由を残すか。 ## 提案 - `font.size_px` は `deserialize_with = "deserialize_flexible_f32"` を追加し、TOML int (`64`) と float (`f64`) の両方を `f32` に正規化。`reference/config.md` の「Must be a TOML float」記述を削除し、example に `14` と `14.0` 両方が valid と追記。`nix/hm-module.nix` の example は `14.0` のままでも int が通ることを担保。 - `validate.rs` に `window_opacity` / `mouse_scroll_multiplier` / `window_backdrop` の warning パスを追加し、`reference/config.md` Behavior 表の Stage 列を `load` (warn) に揃える。`backdrop` の OS 非対応は `window` stage のまま残し、表に注記。 - `theme.palette.indexed` は 1.0 で `String` のまま凍結するなら `reference/config.md` と `felis-config.schema.json` のトップコメントに「TOML key は文字列であるため `indexed` は `String` キー」理由を frozen として記録。`u8` に変えるなら `BREAKING.md` に `base: <sha>` を追加し `just schema` を再生成。どちらかに 1.0 で一意に決める。 ## 判定基準 - `size_px = 14` と `size_px = 14.0` が `felis config check` で共に valid かつ同値として `show-effective` に現れること。 - `window.opacity = 2` / `mouse.scroll_multiplier = -1` が `felis config check` で Warning になること（現行の silent clamp でない）。 - `reference/config.md`「Behavior on missing / malformed values」表と `validate.rs` の warning 対象が 1:1 で対応すること。 - `felis-config.schema.json` の `font.size_px` が `type: number` で int/float 両方を受けること。 ## 対象ファイル - `crates/felis-client-core/src/config.rs` (`FontConfig.size_px`, `WindowConfig.opacity`, `MouseConfig.scroll_multiplier`, `PaletteConfig.indexed`) - `crates/felis-client-core/src/config/validate.rs` (`run`, `font_size` に倣った新関数) - `crates/felis-client-core/felis-config.schema.json` (`just schema`) - `docs/reference/config.md`「Complete annotated example」「Behavior on missing / malformed values」 - `nix/hm-module.nix` (`settings.example`) Parent: #12 および #11 / #28
natsukium commented 2026-09-05 12:11:18 +09:00
Author
Owner
Copy link

Triage (2026-09-05)

Verdict: accept with narrowing, P1. Verified: config.rs:499 documents "Must be a TOML float" as behavior.

  • Item 1 (size_px = 14): do it now. Accept TOML int and float, drop the "must be a float" wording, keep the schema at number.
  • Item 3 (palette.indexed keys): freeze as String and record the reason (TOML keys are strings) in reference/config.md and the schema header. Changing the key type buys nothing the wire does not already own.
  • Item 2 (warnings for opacity / scroll_multiplier clamps, the load vs use stage wording): additive, no now-or-never property, so it lands after v0.1.0 unless it falls out of item 1 for free. Fix the Behavior table's stage column to match the code either way, since that table is frozen prose.
## Triage (2026-09-05) **Verdict: accept with narrowing, P1.** Verified: `config.rs:499` documents "Must be a TOML float" as behavior. - Item 1 (`size_px = 14`): do it now. Accept TOML int and float, drop the "must be a float" wording, keep the schema at `number`. - Item 3 (`palette.indexed` keys): freeze as `String` and record the reason (TOML keys are strings) in `reference/config.md` and the schema header. Changing the key type buys nothing the wire does not already own. - Item 2 (warnings for `opacity` / `scroll_multiplier` clamps, the `load` vs `use` stage wording): additive, no now-or-never property, so it lands after v0.1.0 unless it falls out of item 1 for free. Fix the Behavior table's stage column to match the code either way, since that table is frozen prose.
natsukium commented 2026-09-05 21:26:00 +09:00
Author
Owner
Copy link

Triage plan (2026-09-05)

Source-grounded triage against main at 07a77ae4, reviewed through six rounds of an independent reviewer (pi luna then sol); every finding was verified against the source and folded in, and the design decisions below were settled with the maintainer on 2026-09-05. The order that supersedes the tracker's is posted on #12: this issue lands before #29 (schemas, goldens, and reference tables freeze there).

font.size_px int, clamp warnings, palette.indexed keys

Claim check

  • Item 1 does not reproduce. With the workspace's toml 1.1.5,
    Option<f32> deserializes size_px = 14 to Some(14.0) (checked with
    a standalone program against the same crate version). The "Must be a
    TOML float" sentence (config.rs:499-500) documents a restriction that
    does not exist; the docs example and the HM example are merely
    float-only, not wrong.
  • Item 2 confirmed: validate.rs:57-150 covers font_size,
    fallback_chain, palette, keymap, shader; clamped_opacity
    (config.rs:479) and clamped_scroll_multiplier (:296) clamp at use
    with no diagnostic, while docs/reference/config.md:238 says
    "window.opacity out of range → Clamped … load".
  • Item 3 confirmed: PaletteConfig.indexed: BTreeMap<String, String>
    (config.rs:683), parsed by indexed_slot (:718).

Verdict

accept with narrowing.

  • Item 1: delete the wording; add a parse test pinning that 14 and
    14.0 are equal.
  • Item 3: freeze String keys; record the reason (TOML keys are strings;
    indexed.16 and indexed."16" are one key) in reference/config.md
    and the schema header.
  • Item 2 — Decision (settled 2026-09-05): land the two warnings now. They
    are two ~15-line functions following font_size, and the Behavior table
    row is wrong today and gets edited in this PR either way; shipping the
    table as "clamped at use, silently" freezes prose that describes a gap.
    Alternative (triage): fix the table wording to "window/use" and defer the
    warnings post-release.

Approach

  • config.rs:499-500 wording; validate.rs: window_opacity,
    mouse_scroll_multiplier → DiagnosticKind::Value warning when outside
    the clamp band or non-finite; window.backdrop stays a window-stage
    notice, said so in the table.
  • clamped_scroll_multiplier (config.rs:293-298) is f64::clamp, which
    passes NaN through, and felis-client/src/main.rs:1768-1779 multiplies
    wheel deltas by it, so scroll_multiplier = nan zeroes every notch.
    Give it the same non-finite fallback clamped_opacity has (the default
    multiplier) and test the sanitized value, not only the warning.
  • The stale "must be a float" claim lives in four more places and goes
    with config.rs: nix/hm-module.nix:116-117, docs/how-to/install.md:62,
    .claude/skills/add-config-key/SKILL.md:82, and the reference example.
  • docs/reference/config.md: example shows size_px = 14 accepted;
    Behavior table: opacity row → "Clamped to [0.0, 1.0] with warning,
    load"; add the scroll_multiplier row; note under the table that the
    value diagnostics are 1:1 with validate.rs.
  • Schema: unchanged for size_px (number already admits both); header
    gains the indexed sentence; just schema.
  • CHANGELOG.md: the new warnings (user-visible in config check).
  • Tests: int/float equivalence; each new warning; one negative (in-band
    values produce no diagnostic).

Dependencies / risk / labels

Before #29 (schema header). S. Keep labels.

## Triage plan (2026-09-05) Source-grounded triage against `main` at `07a77ae4`, reviewed through six rounds of an independent reviewer (`pi` luna then sol); every finding was verified against the source and folded in, and the design decisions below were settled with the maintainer on 2026-09-05. The order that supersedes the tracker's is posted on #12: this issue lands before #29 (schemas, goldens, and reference tables freeze there). **`font.size_px` int, clamp warnings, `palette.indexed` keys** ### Claim check - **Item 1 does not reproduce.** With the workspace's `toml 1.1.5`, `Option<f32>` deserializes `size_px = 14` to `Some(14.0)` (checked with a standalone program against the same crate version). The "Must be a TOML float" sentence (`config.rs:499-500`) documents a restriction that does not exist; the docs example and the HM example are merely float-only, not wrong. - Item 2 confirmed: `validate.rs:57-150` covers `font_size`, `fallback_chain`, `palette`, `keymap`, `shader`; `clamped_opacity` (`config.rs:479`) and `clamped_scroll_multiplier` (`:296`) clamp at use with no diagnostic, while `docs/reference/config.md:238` says "`window.opacity` out of range → Clamped … load". - Item 3 confirmed: `PaletteConfig.indexed: BTreeMap<String, String>` (`config.rs:683`), parsed by `indexed_slot` (`:718`). ### Verdict **accept with narrowing.** - Item 1: delete the wording; add a parse test pinning that `14` and `14.0` are equal. - Item 3: freeze `String` keys; record the reason (TOML keys are strings; `indexed.16` and `indexed."16"` are one key) in `reference/config.md` and the schema header. - Item 2 — **Decision (settled 2026-09-05): land the two warnings now.** They are two ~15-line functions following `font_size`, and the Behavior table row is wrong today and gets edited in this PR either way; shipping the table as "clamped at use, silently" freezes prose that describes a gap. Alternative (triage): fix the table wording to "window/use" and defer the warnings post-release. ### Approach - `config.rs:499-500` wording; `validate.rs`: `window_opacity`, `mouse_scroll_multiplier` → `DiagnosticKind::Value` warning when outside the clamp band or non-finite; `window.backdrop` stays a window-stage notice, said so in the table. - `clamped_scroll_multiplier` (`config.rs:293-298`) is `f64::clamp`, which passes NaN through, and `felis-client/src/main.rs:1768-1779` multiplies wheel deltas by it, so `scroll_multiplier = nan` zeroes every notch. Give it the same non-finite fallback `clamped_opacity` has (the default multiplier) and test the sanitized value, not only the warning. - The stale "must be a float" claim lives in four more places and goes with `config.rs`: `nix/hm-module.nix:116-117`, `docs/how-to/install.md:62`, `.claude/skills/add-config-key/SKILL.md:82`, and the reference example. - `docs/reference/config.md`: example shows `size_px = 14` accepted; Behavior table: `opacity` row → "Clamped to `[0.0, 1.0]` with warning, load"; add the `scroll_multiplier` row; note under the table that the `value` diagnostics are 1:1 with `validate.rs`. - Schema: unchanged for `size_px` (`number` already admits both); header gains the `indexed` sentence; `just schema`. - `CHANGELOG.md`: the new warnings (user-visible in `config check`). - Tests: int/float equivalence; each new warning; one negative (in-band values produce no diagnostic). ### Dependencies / risk / labels Before #29 (schema header). **S.** Keep labels.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
ci/darwin-smoke
No results found.
Labels
Clear labels   
priority/P0
Must be resolved before the v0.1.0 tag

  
priority/P1
Must be settled before the first public contract freezes

  
priority/P2
Explicitly deferred until after v0.1.0

  
release/v0.1.0
Work tracked for the first tagged release

  
status/blocked
Cannot start now: waits on an upstream release, another issue, or a recorded revisit trigger

  
status/planned
Triaged with a source-grounded plan and a passed pi review; ready to implement

  
type/bug
Shipped behavior that contradicts the docs or the code's own intent; needs a product code change

  
type/design
An open scope or trade-off question; needs a recorded decision in the owning explanation doc before code

  
type/test-gap
Documented or reviewed behavior that no test pins; usually split from a review finding, needs no product code change

  
type/tracker
Parent issue that tracks child work

No labels
priority/P0
priority/P1
priority/P2
release/v0.1.0
status/blocked
status/planned
type/bug
type/design
type/test-gap
type/tracker
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
natsukium/felis#143
No description provided.