test(core): #297 slice A — RED characterization of the legacy local-search solver - #327
Conversation
) Pre-rename byte/status-shape baseline on current main: - immutable fixture contracts/solve-characterization-v1.json (77 cases: bg x sign x magnitude x hue x chroma x floor; f64 pinned by bits) with anti-vacuum counters — successes both polarities, floored/unfloored, every publicly reachable error class >= 1; - QuantizationGap pinned as publicly EXTINCT on the solid-bg matrix (post-#44 walk always bridges the 8-bit dead zone; ~3.5M-call scan across hue/chroma/floor/vc/gamut found zero) — the variant's truth is pinned at its own seam instead; - solve_many positional property incl. duplicates, empty batch and outer gamut failure staying outer; - cfg(test) probe_log instrumentation: both local searches record every materialized candidate; reports are proven to be drawn ONLY from the examined set; - counterexamples killing the global wording (#297 local-search truth): solve_quantization_neighbor returns QuantizationGap while the 3rd distinct step (outside NEIGHBOR_STEPS) passes in budget — 'nearest' is closest EXAMINED, not global; solve_dj at dJ' 98.75 on white degrades to #000000 (err 1.25) while unexamined #010101 sits strictly closer (err 0.65) BEHIND the one-directional walk; - JND-band characterization vs the independent recheck_against oracle: +-1 budget honoured, reported lc bit-equal to re-measurement, and the tolerant lower acceptance (undershoot within -1 Lc) actually fires (target -7.36 on black accepts #323232 at lc -7.3502); - exposure_quant_and_dj_budgets converted from printed-only to asserted (pinned 39/2121 and 31/2001 flip counts on the current grey grids). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-record (#297) The characterization instrumentation lands inside the admission dependency cone (crates/labcolors-core/src), so the source-bound benchmark must be re-recorded; per the closed-recipe rule the recorder identity is bumped BEFORE recording so the new snapshot covers the already-bumped machinery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cfg(test) instrumentation lands in crates/labcolors-core/src, which sits in the admission benchmark's broad source cone, so the source-bound artifact is re-recorded on the canonical local machine (closed recipe, PASS, 71 mutation self-checks; durable-admission mode). - wcag22-feasibility-benchmark-v6.json: new canonical admission (sha256 2e319643…, benchmark binary 1e43b4c8…, same pinned toolchain c5922366…/fec239e6…); subject drift vs V5 is exactly the recorder identity bump — solve.rs is cone-covered but not an admission subject; - ci.yml: V5 moves to the historical replay chain (snapshot 9819bbd), V6 becomes the current no-worktree validation; immutable sha block extended; - release-contract: V1-V5 frozen byte-exact (V5 hash added), canonical switched to V6, identity projection pinned V6==V5 (provenance may move, the admitted finite algorithm may not). Runtime and compiler WASM bytes are untouched (cfg(test) never ships): size budget stays V6, whole-call boundary artifacts remain bound to their recorded admission versions (append-only, byte-exact). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hostile-review lane findings addressed: - MINOR: the solve_many positional differential now carries the issue-mandated mixed valid/invalid batch (a chroma-ratio job outside [0,1] between valid neighbours must be a positional invalid_input that neither shifts nor poisons position 8) and a separate mid-grey #6E6E6E batch where dark-on-light AA-text is mathematically floor-unreachable (ceiling ~4.14 < 4.5), differential-compared against sequential solve; - NIT: the extinction comment now leads with the structural argument (the -1 Lc acceptance tolerance + QUANT_BUDGET=1 forms a 2-Lc window; wider same-polarity grid windows exist only against the analytic clip, owned by BelowContrastFloor pre-quantisation) and states scan axes honestly (DisplayP3 dies at the outer gate and cannot exercise the search); - NIT: the JND-band docstring no longer calls recheck_against an 'independent oracle' — it is an independent public PATH sharing lpc::contrast_core; the test pins axis consistency, not independent derivation of the metric. Fixture unchanged (the batch tests do not record). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughДобавлены проверки локальности результатов solver и платформенные characterization-контракты. WCAG22 feasibility benchmark обновлён до raw-v6, а CI и release-contract tests теперь проверяют v5 historical replay и текущий v6 artifact. ChangesSolver и characterization
WCAG22 benchmark
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
CI exposed the very thesis of #297 on the first replay: the fixture recorded on macos-aarch64 does not replay bit-for-bit on linux-x64 — the current release is LegacyPlatformDependent, concretely and narrowly: - 74/77 cases are bit-identical across both canonical platforms, including every emitted hex byte, lc, wcag_ratio, floor_override, jp and s bit and every error payload; - the ENTIRE drift is the ulp tail of one field in three cases: the CAM16->Oklab hue correlate h_ok (libm atan2/cbrt differences; 5 ulp on two chromatic mid-grey cases, and an amplified ~1e-8 delta on one achromatic result where hue is degenerate — atan2 of noise against noise). The fixture is therefore pinned PER PLATFORM: - solve-characterization-v1-macos-aarch64.json — recorded on the canonical local machine (renamed from the original single fixture, bytes unchanged); - solve-characterization-v1-linux-x64.json — the canonical CI runner's own rendered observation from the PR #327 replay, which that runner verifies bit-for-bit from now on; - fixture_path() selects by target platform and fails LOUDLY on platforms without a recorded baseline (no silent skip); - platform_fixtures_agree_except_documented_hue_ulp_drift pins the drift exhibit exactly: same 77-case matrix, and the differing (case, field) set must remain precisely the three documented h_ok entries — growth of the exhibit is a numerical behavior change, not new-platform noise. This gives the #290/#292 numerical-provenance work its first committed cross-platform exhibit while keeping the rename-migration invariance gate bit-exact on both CI platforms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
CI-реплей на linux-x64 сработал как задумано и немедленно поймал тезис #297: фикстура macos-aarch64 не реплеится бит-в-бит на Linux. Дрифт хирургический — 3 кейса из 77, только ulp-хвост h_ok (CAM16→Oklab hue; 5 ulp на хроматике + вырожденный ахроматический hue). Всё остальное — hex/lc/wcag_ratio/floor_override/jp/s и все payload'ы ошибок — бит-идентично. Фикстуры теперь платформенные (macos-aarch64 + linux-x64, вторая — дословный вывод самого CI-раннера из этого PR), а сам дрифт запинен тестом platform_fixtures_agree_except_documented_hue_ulp_drift как первый закоммиченный кросс-платформенный экспонат для #290/#292. Push перезапустит CI. |
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 `@crates/labcolors-core/tests/solve_characterization.rs`:
- Around line 350-356: Измените recorder-ветку теста solve characterization: не
перезаписывайте committed fixture, а записывайте наблюдаемый результат в
отдельный candidate-файл. После записи завершайте recorder-run с ошибкой, чтобы
переменная LABCOLORS_RECORD_SOLVE_CHARACTERIZATION, включая значение 0, не могла
скрыть регрессию и дать успешный CI.
In `@scripts/check_wcag22_feasibility_benchmark.py`:
- Around line 31-32: Update DEFAULT_ARTIFACT in the checker to derive its
default directory from tempfile.gettempdir(), matching the benchmark’s
std::env::temp_dir() output location; preserve the existing artifact filename
and avoid hardcoding /private/tmp.
🪄 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: 54842b2d-d8df-4cde-9503-6a129de58fd3
📒 Files selected for processing (9)
.github/workflows/ci.ymlcrates/labcolors-core/benches/wcag22_feasibility_admission.rscrates/labcolors-core/contracts/solve-characterization-v1-linux-x64.jsoncrates/labcolors-core/contracts/solve-characterization-v1-macos-aarch64.jsoncrates/labcolors-core/contracts/wcag22-feasibility-benchmark-v6.jsoncrates/labcolors-core/src/solve.rscrates/labcolors-core/tests/solve_characterization.rspackages/colors/test/release-contract.test.mjsscripts/check_wcag22_feasibility_benchmark.py
CodeRabbit major: the env-gated recorder could silently rebaseline the committed fixture and pass. Adopted the repo's append-only recorder discipline (same as admission/whole-call evidence): - recording refuses to overwrite an existing committed fixture — a rebaseline requires deliberately deleting the file first; - a recording run always ends in a loud panic, so it can never be mistaken for a verified replay; the replay must be rerun without the env var to go green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#297 slice A — RED characterization of the legacy local-search solver
Pre-rename byte/status-shape baseline demanded by #297 («Byte/status-shape characterization happens before rename»). Zero shipped-behavior change: every solve.rs hunk is
cfg(test); runtime/compiler WASM bytes untouched (budget stays V6, whole-call artifacts remain bound to their recorded admission versions).solve / solve_many (issue §RED)
crates/labcolors-core/contracts/solve-characterization-v1.json— 77 cases across bg × sign × magnitude × hue × chroma × floor; hex bytes + f64 payload bits (lc,wcag_ratio,jp,h_ok,s) +floor_override+ every error-variant field. Blind rebaseline forbidden by test docstring; recording env-gated, absent from CI.polarity_mismatch/internal_invariantpinned ==0. All-Err cannot pass.QuantizationGappinned publicly EXTINCT (==0): structurally, the −1 Lc acceptance tolerance +QUANT_BUDGET=1forms a 2-Lc window while wider same-polarity grid windows exist only against the analytic clip (owned byBelowContrastFloorpre-quantisation); empirically corroborated by multi-million-call public-API scans (both polarities, grey+chromatic bgs, hue grid,Relativeup to 1.0, all floors, srgb+dim; DisplayP3 dies at the outer gate). Review lane independently reproduced extinction with a 2.88M-call adversarial scan including AaUi low-target pockets and dense >2 Lc grid-window sweeps.solve_many == positional jobs.map(solve): successes, per-job errors, duplicate jobs, empty batch, mixed valid/invalid batch (invalid chroma job is a positionalinvalid_inputthat neither shifts nor poisons neighbours), per-jobFloorUnreachableon #6E6E6E (dark-side AA ceiling ~4.14 < 4.5) differential-compared to sequential, and the outerGamutUnsupportedstaying a whole-batch outer error.recheck_against(independent public path, same measurement core — axis consistency, honestly framed): ±1 budget held, reportedlcbit-equal to re-measurement, and the tolerant lower acceptance pinned as REAL — target −7.36 on black accepts#323232at lc −7.3502 (undershoot 0.0098 within the −1 tolerance). «Решено» на этой поверхности значит «в допуске», а не «на-или-за целью» — честные имена обязаны это проговорить.local-search truth (issue §local-search truth)
cfg(test)probe_log: both local searches record every materialized on-grid candidate; tests prove every report (returned colour, gapnearest, degradedachieved_dj) is drawn ONLY from the examined set, and the walk never examines more than seed +NEIGHBOR_STEPS.QuantizationGap/nearestwording at its own seam: on the real hex grid with an injected measurement law, the walk (seed + 2 distinct neighbours) returnsQuantizationGapwhile the 3rd distinct step — the first point the search is forbidden to visit — passes in budget;nearestequals the closest EXAMINED, not the global nearest.solve_dj(#FFFFFF, 98.75, +1)degrades to#000000(achieved 100.0, err 1.25) with the ENTIRE examined set being that one colour, while unexamined same-polarity#010101achieves 98.0964 (err 0.65) — strictly closer, behind the one-directional walk.GlobalOverDeclaredDomain/ProvenInfeasible— the matrix pins exact terminal classes.numerical/claim hygiene (issue §numerical/claim hygiene)
exposure_quant_and_dj_budgetsconverted from printed-only to asserted (pinned flip counts 39/2121 Lc and 31/2001 dJ'), scoped to the current 8-bit grey readability grids with an explicit no-perceptual/universal-claim disclaimer. Names indocs/empirical-inventory.mdrows 50–51 unchanged.Evidence
identityProjection(V6)==identityProjection(V5)(provenance may move, the admitted finite algorithm may not).Review lanes (already run)
Local gates: core suite (597 lib + all integration), workspace
--locked, clippy-D warningsparity, rustfmt, npm 143/0, docs-drift 25/0, naming inventory, admission V6 durable PASS.GREEN phase (rename migration Rust→WASM/TS→npm→conformance→Swift→docs, new truthful status family,
LocalCandidateSearchReportV1semantics incl. whether bisection probes count as examined) lands as the next slice per the issue's atomic-migration plan.🤖 Generated with Claude Code
Summary by CodeRabbit
Тесты
Инфраструктура
Надёжность