C5.1: клиентские theme-ключи → VcPreset (снос fixed Theme из рантайма) - #343
Conversation
Канонический путь: client key → словарь themes загруженного конфига → VcPreset → ViewingConditions. Engine хранит иммутабельные key→preset биндинги; ключ кэша несёт СЛОТ биндинга (без аллокации), отпечаток конфига разводит словари; результат сохраняет исходный клиентский ключ. Resolve и recheck одинаково требуют конфиг (recheck без конфига был дырой — теперь ConfigRequired); неизвестный ключ и любой ключ при пустом словаре — типизированный UnknownTheme. Успешный reload чистит result-кэш и projection-memo только после полной валидации; неудачный не трогает state/cache/memo (пин-тест). Вырезано: wasm-модуль theme.rs (fixed parse), TS-union ThemeName (теперь string — ключ клиентского словаря), контекст-мост legacy-прокси в cleanliness.rs (enum Theme, DefectContext, muddiness_in_context, drab_in_context, vc_for_context, y_pct_from_hex + их Zone-G тесты — это машинерия fixed-theme, её место занял канонический путь; замороженная координата muddiness_from_hex/oklch остаётся до C5.2). Новый публичный Core Theme не создан: физический тип — VcPreset. FFI и conformance держат ЛОКАЛЬНЫЕ adapter/fixture словари (маппинг своих четырёх ключей в VcPreset) — байты пака не тронуты. Пять новых контракт-тестов C5.1 в engine.rs (клиентские имена, два ключа одного пресета, пустой словарь, recheck-требует-конфиг, атомарный reload). Гейты: workspace-тесты (37 таргетов), clippy -D warnings, fmt, MSRV 1.85 check, rustdoc -D warnings, tsc, npm 169/169. Runtime-wasm байты изменятся (лукап вместо enum) — budget v12 по каноническому CI-размеру следующим коммитом. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughВстроенные имена тем удалены: WASM использует ключи клиентского конфига, сопоставляет их с ChangesГраница клиентских тем
WASM-бюджет v12
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Канонический Linux-x64 размер снят draft-CI (run 29606827872): словарь клиентских ключей вместо fixed enum стоит +416B. v11 уходит в неизменяемую историю с SHA-пином; ratchet v12 = точное измерение, zero headroom. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
theme_vc парити-теста маппит kebab-ключи labui-паспорта в VcPreset напрямую — локальный fixture-словарь вместо удалённого core Theme. Таргет wasm32 не покрывался локальным check --all-targets, поймано draft-CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Без конфига словаря тем не существует — resolve отвечает config_required; unknown_theme проверяется с загруженным labui-паспортом (ключ вне словаря). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ст, доки
- Пустой словарь themes теперь отклоняется НА ЗАГРУЗКЕ (ConfigError::EmptyThemes)
— симметрия с EmptyContract у ролей: поздний unknown_theme был неотличим от
опечатки; прежнее состояние движка не тронуто (тест).
- Тест на дубликат имени темы (mutation-bite для check_unique("themes") —
единственный непокушенный мутант новой поверхности).
- Стейл-док «all four spellings are fully supported» над ThemeName удалён
(утекал потребителям в published d.ts).
- Вакуумная ассерция invalid-fg-hex в recheck-тесте: теперь с загруженным
конфигом и точным матчем InvalidBackground.
- Сообщения границы: UnknownTheme на английском (язык поверхности),
ConfigRequired упоминает recheck (новая достижимость).
- README: recheckContrast требует конфиг (явно, не только в «Темах»);
строка unknown_theme в таблице ошибок; ADR-0001 приведён к C5.1
(fixed-enum вырезан, канон — словарь конфига).
- wasm_parity: стейл-коммент «recheck is stateless» заменён правдой.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Допилы ревью (EmptyThemes на загрузке + строки границы) стоили +108B поверх словаря-лукапа. v12 ещё не в неизменяемой истории (тот же PR) — перепин на месте по прецеденту v10; канонический размер снят текущим draft-CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/labcolors-wasm/src/engine.rs (1)
139-163: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winСначала проверяйте конфиг и ключ темы.
Сейчас невалидный
bg_hexвозвращаетinvalid_backgroundдаже без конфига или при неизвестной теме. Это противоречит документированномуconfig_requiredдоloadConfigи отличается от порядкаrecheck_vc. Перенесите нормализацию фона послеtheme_binding.Предлагаемое исправление
- let normalised = normalise_hex(bg_hex)?; - let bg = BgInput::solid(&normalised).map_err(|u| BindingError::InvalidBackground { - reason: u.to_string(), - })?; - if let Some(named) = &self.named { let (slot, preset) = named .theme_binding(theme_key) .ok_or_else(|| BindingError::UnknownTheme { requested: theme_key.to_string(), })?; + let normalised = normalise_hex(bg_hex)?; + let bg = + BgInput::solid(&normalised).map_err(|u| BindingError::InvalidBackground { + reason: u.to_string(), + })?; let vc = preset.viewing_conditions();As per coding guidelines, «Проверяйте hostile input, порядок событий и reentrancy там, где применимы соответствующие риски».
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/labcolors-wasm/src/engine.rs` around lines 139 - 163, In resolve_theme, validate the loaded configuration and resolve theme_binding(theme_key) before normalising bg_hex or constructing BgInput, so missing configuration or an unknown theme returns its existing error regardless of background validity. Move the normalise_hex and BgInput validation block after the named/theme-binding checks while preserving the cache-key and resolved-theme flow.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/labcolors-core/src/config.rs`:
- Around line 1282-1287: Обновите публичный `# Errors` doc-комментарий у метода,
содержащего проверку `self.themes.entries.is_empty()`, добавив
`ConfigError::EmptyThemes` наряду с `ConfigError::EmptyContract`. Не изменяйте
логику проверки или другие варианты ошибок.
---
Outside diff comments:
In `@crates/labcolors-wasm/src/engine.rs`:
- Around line 139-163: In resolve_theme, validate the loaded configuration and
resolve theme_binding(theme_key) before normalising bg_hex or constructing
BgInput, so missing configuration or an unknown theme returns its existing error
regardless of background validity. Move the normalise_hex and BgInput validation
block after the named/theme-binding checks while preserving the cache-key and
resolved-theme flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 24bf1032-2986-4bcb-9e11-4e8021a48464
📒 Files selected for processing (21)
conformance/README.mdcrates/labcolors-conformance/src/lib.rscrates/labcolors-core/src/cleanliness.rscrates/labcolors-core/src/config.rscrates/labcolors-core/src/config/tests.rscrates/labcolors-core/src/lib.rscrates/labcolors-core/tests/property_invariants.rscrates/labcolors-ffi/src/lib.rscrates/labcolors-wasm/src/cache.rscrates/labcolors-wasm/src/dto.rscrates/labcolors-wasm/src/engine.rscrates/labcolors-wasm/src/error.rscrates/labcolors-wasm/src/lib.rscrates/labcolors-wasm/src/projection.rscrates/labcolors-wasm/src/theme.rscrates/labcolors-wasm/tests/wasm_parity.rsdocs/decisions/0001-config-boundary.mdpackages/colors/README.mdpackages/colors/bench/wasm-size-budget-v12.jsonpackages/colors/test/release-contract.test.mjsscripts/check-wasm-size-budget.mjs
💤 Files with no reviewable changes (2)
- crates/labcolors-wasm/src/theme.rs
- crates/labcolors-core/src/cleanliness.rs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Роадмап §16 C5.1 — первый из двух PR среза C5 (следом C5.2: вырез legacy cleanliness-прокси).
Что сделано
client key → themes-словарь конфига → VcPreset → ViewingConditions. Fixed-словарьlight/dark/*-icиз рантайма вырезан; встроенных имён тем у движка больше нет.unknown_theme; пустой словарьthemesотклоняется на загрузке (EmptyThemes— симметрия сEmptyContractу ролей: позднийunknown_themeбыл бы неотличим от опечатки).theme.rs, TS-unionThemeName(теперьstring), контекст-мост вcleanliness.rs. Scope-заметка:DefectContext/*_in_context/vc_for_context/y_pct_from_hexчислятся в списках C5.2, но вырезаны здесь — они параметризованы удаляемымTheme, переписывать их наVcPresetради нескольких часов жизни было бы хуже; замороженная координатаmuddiness_from_hex/oklchостаётся до C5.2. Новый публичный CoreThemeне создан (физический тип —VcPreset).Байты
Budget v12: 459657B (+416B — словарь-лукап вместо enum), канонический размер снят draft-CI (run 29606827872), подтверждён зелёным прогоном; v11 — в неизменяемой истории.
Ревью
Hostile-ревью (read-only, полный прогон гейтов): GO; оба обязательных допила (стейл-док ThemeName в published d.ts, тест дубликата имени темы) и все минорки внесены, включая выравнивание пустого словаря под прецедент EmptyContract.
Локальные гейты
Workspace-тесты (37 таргетов), clippy
-D warnings, fmt, MSRV 1.85 check, rustdoc-D warnings, wasm32-check тестов, tsc, npm 169/169 — зелёные.🤖 Generated with Claude Code
Summary by CodeRabbit