C8d(4): packed WASM recheck boundary + typed verdict + numeric theme handle (hard-cut string overloads, semver-major) - #407
Merged
Conversation
WalkthroughДобавлен packed API для проверки контраста: цвета передаются как ChangesPacked contrast recheck
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant adaptTheme
participant themeArgFor
participant WASMEngine
participant ThemeDictionary
adaptTheme->>themeArgFor: resolve and cache theme handle
themeArgFor-->>adaptTheme: numeric theme handle
adaptTheme->>WASMEngine: recheckContrastMulti(bgs, fgs, theme handle)
WASMEngine->>ThemeDictionary: resolve ViewingConditions by handle
ThemeDictionary-->>WASMEngine: return ViewingConditions
WASMEngine-->>adaptTheme: flat contrast results
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: read ECONNRESET Comment |
…handle (hard-cut string overloads) Replace the Vec<String> recheck overloads with the ratified Option A packed wire (F1): recheckContrast(bg:u32 0x00RRGGBB, fgs:Uint32Array, theme:number) and recheckContrastMulti(bgs:Uint32Array, fgs:Uint32Array, theme:number), each one contiguous typed-array copy into linear memory — zero hex parse, zero String/Cow per foreground, reserved high byte validated once without alloc. The interleaved / background-major Float64Array OUTPUT layout stays frozen and byte-identical to the golden (core recheck_against_u32/_multi_u32 from step 1). Mint a numeric theme handle (themeHandle) so the stateless recheck path (F2) addresses the theme dictionary by slot instead of re-scanning it by key every frame; the begin/observe/end Session lifecycle stays deferred. Hard-cut the string overloads (F3, semver-major): engine.rs recheck/recheck_multi /recheck_vc removed in favour of recheck_u32/recheck_multi_u32/recheck_vc_by_handle + theme_handle; wasm boundary annotations regenerate the .d.ts. Internal consumers migrated mechanically to the packed input in the same commit — adaptTheme packs samples/foregrounds and mints the handle at the cold recheck seam (worst->full- support algorithm unchanged, that is step 5) — and every affected test. FFI stays on the string path (C8e/F6). resolveTheme wide-JSON output untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LzZyjuo5ahzmW6V8pPJqSJ
lemone112
force-pushed
the
claude/color-engine-semantic-drift-jadrtr
branch
from
July 21, 2026 15:45
46b2e9c to
30513b8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/colors/adapt-theme.js`:
- Around line 310-324: Validate the result of mintThemeHandle(themeName) inside
themeArgFor before caching or returning it. Reject undefined, NaN, strings, and
other non-numeric or non-finite handles with a clear adaptTheme diagnostic,
while preserving the existing memoization and raw-theme fallback when
mintThemeHandle is unavailable.
- Around line 310-324: Update themeArgFor so the newly minted handle remains
local until checkpoint(owner) completes successfully; only then publish both
themeArgValue and themeArgKey together. Preserve the existing memoization and
raw-theme fallback behavior, ensuring cancellation or reentrant callbacks cannot
leave the key and handle out of sync.
🪄 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: 562e0642-7256-46f5-9e8f-7cf13b5c9890
📒 Files selected for processing (12)
crates/labcolors-wasm/src/engine.rscrates/labcolors-wasm/src/lib.rscrates/labcolors-wasm/tests/wasm_parity.rspackages/colors/README.mdpackages/colors/adapt-theme.d.tspackages/colors/adapt-theme.jspackages/colors/bench/hotpath.bench.mjspackages/colors/bench/wasm-boundary.bench.mjspackages/colors/test/adapt-theme.test.mjspackages/colors/test/chain-invariants.test.mjspackages/colors/test/hotpath-parity.test.mjspackages/colors/test/wasm-boundary-parity.test.mjs
💤 Files with no reviewable changes (1)
- packages/colors/bench/hotpath.bench.mjs
Comment on lines
+310
to
+324
| // Numeric theme-handle memo. Mint at most once per distinct theme key; the | ||
| // recheck loop then passes the numeric handle (or the raw key, when the engine | ||
| // exposes no themeHandle capability). | ||
| let themeArgKey = null; | ||
| let themeArgValue = null; | ||
| const themeArgFor = (themeName, owner) => { | ||
| if (!mintThemeHandle) return themeName; | ||
| if (themeName !== themeArgKey) { | ||
| themeArgValue = mintThemeHandle(themeName); | ||
| checkpoint(owner); | ||
| themeArgKey = themeName; | ||
| } | ||
| return themeArgValue; | ||
| }; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Возвращаемое значение mintThemeHandle не валидируется.
В отличие от recheckContrast/recheckContrastMulti (проверка длины буфера, финитности значений) и isStableGlowPointNoop (проверка typeof noop !== "boolean"), результат mintThemeHandle(themeName) никак не проверяется перед передачей в recheckContrast/recheckContrastMulti. Некорректное значение (например, undefined, NaN, строка) даст малопонятную ошибку уже на границе WASM вместо чёткой диагностики adaptTheme.
🤖 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 `@packages/colors/adapt-theme.js` around lines 310 - 324, Validate the result
of mintThemeHandle(themeName) inside themeArgFor before caching or returning it.
Reject undefined, NaN, strings, and other non-numeric or non-finite handles with
a clear adaptTheme diagnostic, while preserving the existing memoization and
raw-theme fallback when mintThemeHandle is unavailable.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Мутация кэша themeArgKey/themeArgValue происходит до checkpoint — риск рассинхронизации при отменённой операции.
themeArgValue перезаписывается ДО checkpoint(owner), а themeArgKey — только после. Если mintThemeHandle (внешний колбэк потребителя, потенциально реентрантный — как и остальные capability-колбэки в этом файле) успевает инициировать более новую операцию до того, как checkpoint бросит CANCELLED, кэш остаётся в несогласованном состоянии: themeArgKey указывает на старую тему, а themeArgValue уже содержит handle для ДРУГОЙ темы. Следующий вызов с тем же старым themeName вернёт чужой handle без какой-либо ошибки — тихая порча состояния, нарушающая тот же инвариант «публикуем состояние только после подтверждения владения», который соблюдается везде в остальном файле (applyHexes, commitResolved и т.п.).
🔒️ Предлагаемый фикс: не публиковать состояние до checkpoint
const themeArgFor = (themeName, owner) => {
if (!mintThemeHandle) return themeName;
if (themeName !== themeArgKey) {
- themeArgValue = mintThemeHandle(themeName);
- checkpoint(owner);
- themeArgKey = themeName;
+ const minted = mintThemeHandle(themeName);
+ checkpoint(owner);
+ themeArgValue = minted;
+ themeArgKey = themeName;
}
return themeArgValue;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Numeric theme-handle memo. Mint at most once per distinct theme key; the | |
| // recheck loop then passes the numeric handle (or the raw key, when the engine | |
| // exposes no themeHandle capability). | |
| let themeArgKey = null; | |
| let themeArgValue = null; | |
| const themeArgFor = (themeName, owner) => { | |
| if (!mintThemeHandle) return themeName; | |
| if (themeName !== themeArgKey) { | |
| themeArgValue = mintThemeHandle(themeName); | |
| checkpoint(owner); | |
| themeArgKey = themeName; | |
| } | |
| return themeArgValue; | |
| }; | |
| // Numeric theme-handle memo. Mint at most once per distinct theme key; the | |
| // recheck loop then passes the numeric handle (or the raw key, when the engine | |
| // exposes no themeHandle capability). | |
| let themeArgKey = null; | |
| let themeArgValue = null; | |
| const themeArgFor = (themeName, owner) => { | |
| if (!mintThemeHandle) return themeName; | |
| if (themeName !== themeArgKey) { | |
| const minted = mintThemeHandle(themeName); | |
| checkpoint(owner); | |
| themeArgValue = minted; | |
| themeArgKey = themeName; | |
| } | |
| return themeArgValue; | |
| }; |
🤖 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 `@packages/colors/adapt-theme.js` around lines 310 - 324, Update themeArgFor so
the newly minted handle remains local until checkpoint(owner) completes
successfully; only then publish both themeArgValue and themeArgKey together.
Preserve the existing memoization and raw-theme fallback behavior, ensuring
cancellation or reentrant callbacks cannot leave the key and handle out of sync.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/colors/adapt-theme.js`:
- Around line 151-161: Make the numeric theme-handle capability mandatory in the
initialization logic around themeHandleCapability and mintThemeHandle: reject
colors immediately when themeHandle is missing, while retaining the existing
TypeError for non-function values. Remove the null/string fallback path so all
later recheckContrast calls receive a numeric theme handle.
🪄 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: 37438b93-1981-4e75-a0c7-d70b7a4cd685
📒 Files selected for processing (14)
crates/labcolors-wasm/src/engine.rscrates/labcolors-wasm/src/lib.rscrates/labcolors-wasm/tests/wasm_parity.rspackages/colors/README.mdpackages/colors/adapt-theme.d.tspackages/colors/adapt-theme.jspackages/colors/bench/hotpath.bench.mjspackages/colors/bench/wasm-boundary.bench.mjspackages/colors/bench/wasm.jsonpackages/colors/test/adapt-theme.test.mjspackages/colors/test/chain-invariants.test.mjspackages/colors/test/hotpath-parity.test.mjspackages/colors/test/wasm-boundary-parity.test.mjsscripts/check-wasm-size-budget.mjs
💤 Files with no reviewable changes (1)
- packages/colors/bench/hotpath.bench.mjs
Comment on lines
+151
to
+161
| // Optional numeric theme handle (like recheckContrastMulti, it is an engine | ||
| // capability the controller uses when offered). When present, a theme key is | ||
| // lowered to its numeric handle ONCE per distinct theme at a cold recheck | ||
| // edge, then addressed numerically — the hot loop never re-scans the theme | ||
| // dictionary by string. Engines without it keep the string theme key. | ||
| const themeHandleCapability = colors.themeHandle; | ||
| if (themeHandleCapability !== undefined && typeof themeHandleCapability !== "function") { | ||
| throw new TypeError("adaptTheme: themeHandle must be a function"); | ||
| } | ||
| const mintThemeHandle = | ||
| typeof themeHandleCapability === "function" ? themeHandleCapability.bind(colors) : null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Не передавайте строковый fallback в numeric-only ABI.
recheckContrast уже объявлен с theme: number, а string overload удалён. При отсутствии themeHandle здесь передаётся строка, поэтому incomplete capability contract падает лишь поздно на WASM boundary. Сделайте themeHandle обязательным и отклоняйте такой colors сразу.
Предлагаемое исправление
- colors: Pick<LabColors, "resolveTheme" | "recheckContrast"> &
- Partial<Pick<LabColors, "recheckContrastMulti" | "themeHandle" | "isStableGlowPointNoop">>;
+ colors: Pick<LabColors, "resolveTheme" | "recheckContrast" | "themeHandle"> &
+ Partial<Pick<LabColors, "recheckContrastMulti" | "isStableGlowPointNoop">>;- if (themeHandleCapability !== undefined && typeof themeHandleCapability !== "function") {
+ if (typeof themeHandleCapability !== "function") {
throw new TypeError("adaptTheme: themeHandle must be a function");
}- if (!mintThemeHandle) return themeName;Как предписывает coding guidelines: «Новый или изменяемый public path не должен … получать plausible fallback; … incomplete context должны возвращаться типизированно».
Also applies to: 315-316
🤖 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 `@packages/colors/adapt-theme.js` around lines 151 - 161, Make the numeric
theme-handle capability mandatory in the initialization logic around
themeHandleCapability and mintThemeHandle: reject colors immediately when
themeHandle is missing, while retaining the existing TypeError for non-function
values. Remove the null/string fallback path so all later recheckContrast calls
receive a numeric theme handle.
Source: Coding guidelines
This was referenced Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Слайс C8d, шаг 4 — публичная граница WASM (semver-major)
Стек на влитых шагах 1-3 (#404/#405/#406). Первый не-core-only шаг: заменяет строковую
Vec<String>recheck-границу на packed numeric wire. Пакет@labpics/colorsпока 0.11.0 (pre-1.0), так что major-слом уместен.Ратификация форков (видимый DAG-лог)
Отдельная ratify-фаза воркфлоу сверила три
needs_ratificationфорка с роадмап-SSOT — все RATIFIED с цитатами:Uint32ArrayRGB24 (0x00RRGGBB, старший байт required-zero) + скалярный numeric theme handle. Опора: §12 строка 1788 «packed typed-array batch», §16 C8d строки 2600-2601 «Vec<String>… заменяется одним numeric-handle/packed typed-array batch-ем».Что сделано
recheckContrast(bg:number 0x00RRGGBB, fgs:Uint32Array, themeHandle:number):Float64ArrayиrecheckContrastMulti(bgs:Uint32Array, fgs:Uint32Array, themeHandle:number):Float64Array(lib.rs + engine.rs), маршрутизированы через corerecheck_against_u32/_multi_u32(шаг 1) — один contiguous-копи, ноль hex-parse и строковых аллокаций на update-пути. Старший байт валидируется один раз без аллокации.engine.themeHandle(theme):number): ключ темы разрешается один раз на холодном крае, в горячем цикле адресуется числом (F2, stateless).Verified|Violation{worstSample,worstRole,provenance}+ packed readout..d.tsгенерится wasm-bindgen, публичную поверхность enforce-ятpublic-claims/public-api-cleanup/release-contractтесты (обновлены под packed surface, один публичный корень).adapt-theme.jsмигрирован механически на packed вход (packRgb24Hex+ мит handle); алгоритм worst→full-support НЕ тронут — это шаг 5.Заморожено (byte-identity держится)
Float64ArrayOUTPUT layout (interleaved / background-major) иresolveThemewide-JSON hex — не изменены.wasm-boundary.golden.jsonпуст в диффе. Тесты C1 (byte-identity к frozen golden через packed вход) и C2 (background-major layout = N per-sample вызовов) закрепляют это в CI. FFI не тронут (остаётся строковым — F6, C8e).Ревью (adversarial) + применённые фиксы
3 high-effort read-only линзы → синтез READY_WITH_FIXES. Применил все находки:
pkне разворачивал#RGBshorthand, а frozen golden содержит shorthand-фикстуры (#fff/#123) — CI покраснел бы. Fix:pkтеперь зеркалитpackRgb24Hex(разворот shorthand); golden не тронут. Проверено на всех golden-кейсах.bench/wasm-boundary.bench.mjsкормил hex-строки в packed ABI (wasm-bindgen молча коэрсил в 0 → бенч мерил мусор). Fix: мигрирован на packed (themeHandle+Uint32Array).bench/hotpath.bench.mjsбыл уже мёртв (импорт удалённогоeffectiveBackground), не мигрируем без несвязанной работы → удалён (не в CI/scripts; реальную границу меритwasm-boundary.bench.mjs).themeHandle.Гейты (локально, зелёные где применимо)
cargo build -p labcolors-wasm --target wasm32-unknown-unknown,cargo test -p labcolors-wasm(74),cargo test -p labcolors-core(606),cargo fmt --all --check,cargo clippy --workspace --all-targets -D warnings,cargo doc -D warnings,tsc --noEmit,node --testpure-JS (89 на затронутых). C1/C2 byte-identity и размер валидирует CI послеwasm-pack build(wasm-pack локально недоступен).Ожидаемо: размер бинарника изменится
Удаление строкового пути меняет codegen → size-джоба сообщит новый
rawBytes.bench/wasm.json+WASM_BUDGET_FILE_SHA256НЕ тронуты в этом коммите — честный re-baseline на фактические байты CI сделаю сразу после первого прогона (дисциплина C3b).🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
Новые возможности
themeHandle.0x00RRGGBB.Изменения API
recheckContrast/recheckContrastMultiпереведены на упакованные входы иthemeHandleвместо строковых hex и строки темы (старые строковые варианты удалены).Устойчивость / Bug Fixes
Документация