Proof: bind what each comparator coordinate contains, not only its digest (V5b2d-4i) - #559
Conversation
…gest (V5b2d-4i) The manifest bound ten coordinate *numbers*. Nothing held the preimages those numbers fold. A domain separator inside the derivation could be rewritten, and two coordinates could be swapped, with the whole 408-test proof set staying green: measured on this tree, flipping "ordered admitted legal-file set; no legal-compliance claim" left the 267-test arb gate and the 408-test shared suite at their baseline, and swapping engine_release with upstream_source left all 58 test_pipeline contracts green. The meaning of a coordinate was therefore redefinable in silence, while comparator identity is the foundation of the decision chain. test_comparator_content.py binds the INPUT of the fold, in three independent layers over one deterministic derivation (synthetic source closure, mocked build backend, admitted local sources): - naming law: the coordinate named X must carry the domain separator that spells X, decoded by an independent wire-format oracle rather than by the pipeline encoder. Kills a swap of any two coordinates; - reference vector: exact preimage bytes of all eight source-bound coordinates, keyed by coordinate NAME. Kills any byte change, including separator edits and reorderings the structural layer cannot see. The covered set is derived from protocol.source_bound_coordinates_v2(), so a new source-bound coordinate cannot land without its own bytes; - structural golden: every preimage decoded to (separator, version, ordered chunks) and rendered without admitted source bytes — a file body prints as file:<path>. Editing interval.c cannot move this golden, so any movement of it is semantic and regenerating it for a green CI is not a thing that happens. Layers do not duplicate the existing pins: file contents stay bound by _PINNED_BUILD_SOURCE_SHA256_V1, the build-process encoding stays bound by its own golden in test_build.py. The two BUILD-observation coordinates are outside the byte vector for that reason and are fully covered structurally. Both inventory pins updated from an executed gate: 267 -> 275 tests.
- Docstring no longer overclaims: the swap class was open for the engine_release/upstream_source pair (wrapper/evaluator was already caught by a pre-existing test); the structural golden pins only chunk order and count for BUILD coordinates and only presence/position for chunks over 64 bytes. - Failure paths no longer dump megabyte objects: the derived-comparator guard raises type name + repr sha256, and the decoder replay test compares preimage hashes and names the coordinate.
Both inventory pins conflicted by construction: this branch adds the eight comparator-content tests, main added its own. Neither side's value is the union, so both were recomputed by executing the gate's own enumeration: 292 tests (284 from main + 8 here). - proof/region/v1/arb/tests/gate.py: EXPECTED_TEST_INVENTORY_SHA256 - proof/region/v1/tests/test_build.py: ARB_INVENTORY/ORDER_SHA256 + COUNT (kept an independent outer oracle; recomputed, not imported)
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 40 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 (2)
WalkthroughДобавлены независимые тесты содержимого Arb-компаратора. Архивация tar-данных переведена на канонический gzip-формат. Обновлены эталонные хеши, идентичности и инвентарь тестов. ChangesЦелостность Arb-компаратора
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
The docstring claimed the layers do not overlap — pinned source bytes there, coordinate meaning here. A mutation falsified it: editing `interval.c` and correctly repinning `_PINNED_BUILD_SOURCE_SHA256_V1` still reddens the `wrapper_source` reference vector, because the vector folds the admitted files transitively. Whoever edits an evaluator source needs both repins, and the docstring now says so. The structural golden stays green through that edit, which is the point of its coarseness: it cannot be regenerated into agreement.
|
Замечания независимой проверки разобраны. F1 — принято, закрыто в F2 — подтверждаю находку, но её починка сюда не влезает, и вот почему.
Правильное место — непривилегированный Смягчающее, чтобы масштаб был понятен: внешний оракул в |
|
Не вливаю. CI красный, и причина содержательная. Четыре координаты reference-вектора расходятся между Python 3.12 и 3.14.6: Воспроизведено в точно закреплённом образе Механизм. Это дефект теста, а не закона идентичности. Производственное кодирование замка ( Как чинить — и как НЕ чинить. Не пересчитывать числа под 3.14: это воспроизведёт тот же дефект при следующем сдвиге пина версии. Убрать зависимость: либо закрепить байты фикстурного архива явно (положить готовый архив/его точные байты), либо строить reference-вектор над входами, не проходящими через Замечу, что нашёл это тот самый гейт, который сегодня переехал в CI-контейнер (#562/#563). До этого |
… (V5b2d-4i) test_pipeline._tar() synthesized the admitted source archives with gzip.compress(compresslevel=9). Deflate output is not stable across zlib versions: Python 3.12.3 (zlib 1.2.13) and the pinned CI image python:3.14.6-slim (zlib 1.3.1) emit different bytes for identical input, which moved archive_sha256 -> source_lock_identity -> the four archive-derived source-bound preimages (engine_release, upstream_source, arithmetic_input_set, legal_file_set). The raw ustar stream was proven identical across versions, so only the container drifted. The fixture now encodes archives as a canonical RFC 1951 stored-deflate gzip member whose bytes are a pure function of the raw tar stream and cannot vary with the interpreter's zlib. A regression test (FixtureArchiveDeterminismTests) rebuilds the canonical member independently and reddens on a return to gzip.compress. Pins recomputed by executing the gates in the pinned container: SOURCE_BOUND_PREIMAGE_SHA256_V1 (4 archive-derived coordinates), arb gate inventory (293 tests) in gate.py and test_build.py, and the build-process characterization pins in test_build.py (input_bundle_identity, process_bytes sha256, comparator/evidence/claim identities).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
proof/region/v1/tests/test_build.py (1)
313-332: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winОбоснуйте перегенерацию пинов
build_process_bytes_v1. Оба сайта закрепляют sha256 одного объекта —build_process_bytes_v1отbuild_processes[0]. Значение обновлено в обоих местах, но длина кодировки (196 байт) и sha256 входного набора сборки не изменились. Общий корень: неясно, связывают ли эти величины замок источников, чьиarchive_sha256изменились из-за перехода на канонический stored-deflate gzip.
proof/region/v1/tests/test_build.py#L313-L332: подтвердите, чтоinput_bundle_identityиbuild_process_bytes_v1связывают замок источников, а не только входной набор сборки; при подтверждении зафиксируйте эту причину комментарием рядом с пином.proof/region/v1/tests/test_build.py#L2446-L2455: после подтверждения оставьте значение согласованным со строкой 332; расхождение между двумя пинами означает ошибку перегенерации.🤖 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 `@proof/region/v1/tests/test_build.py` around lines 313 - 332, В proof/region/v1/tests/test_build.py#L313-L332 подтвердите, что input_bundle_identity и build_process_bytes_v1 связывают именно замок исходников, а не только входной набор сборки, и добавьте рядом с пином комментарий с этой причиной перегенерации после перехода на канонический stored-deflate gzip; в proof/region/v1/tests/test_build.py#L2446-L2455 оставьте значение build_process_bytes_v1 согласованным с пином на строке 332, исправив расхождение при необходимости.
🤖 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 `@proof/region/v1/arb/tests/test_comparator_content.py`:
- Around line 361-371: Импортируйте functools и добавьте декоратор
`@functools.cache` к функции _derived_comparator_v1, сохранив её текущую сигнатуру
и поведение; повторные вызовы должны переиспользовать единственный результат
вместо повторного запуска ControlledPipelineV1.build.
---
Outside diff comments:
In `@proof/region/v1/tests/test_build.py`:
- Around line 313-332: В proof/region/v1/tests/test_build.py#L313-L332
подтвердите, что input_bundle_identity и build_process_bytes_v1 связывают именно
замок исходников, а не только входной набор сборки, и добавьте рядом с пином
комментарий с этой причиной перегенерации после перехода на канонический
stored-deflate gzip; в proof/region/v1/tests/test_build.py#L2446-L2455 оставьте
значение build_process_bytes_v1 согласованным с пином на строке 332, исправив
расхождение при необходимости.
🪄 Autofix
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: 5b3db282-b39d-4286-8625-8d3d326ca4af
📒 Files selected for processing (4)
proof/region/v1/arb/tests/gate.pyproof/region/v1/arb/tests/test_comparator_content.pyproof/region/v1/arb/tests/test_pipeline.pyproof/region/v1/tests/test_build.py
…igin CodeRabbit CHANGES_REQUESTED on head c30a885: _derived_comparator_v1 rebuilt the whole ControlledPipelineV1 comparator on every call, and the re-derived input_bundle_identity/build_process_bytes pins were unexplained. - test_comparator_content.py: decorate _derived_comparator_v1 with functools.cache; the input is deterministic (same fixture closure), so repeated calls now reuse one build observation instead of re-running the pipeline. - test_build.py: comment at the pins — input_bundle_identity folds request.admitted_sources.identity (which commits to each archive_sha256), so re-encoding the fixture archives as canonical stored-deflate gzip members legitimately moved it; input_bundle_sha256 (the USTAR contents hash) is unchanged and proves only the source-lock binding moved, not the bundle bytes.
1 participant
Что это
Срез закрепляет содержимое координат Arb-компаратора, а не только их числа.
Манифест связывал
sha256от преимиджа. Сам преимидж не удерживал никто:домен-сепаратор внутри деривации можно было переписать, а две координаты —
поменять местами, и весь набор оставался зелёным.
Новый модуль
proof/region/v1/arb/tests/test_comparator_content.py(8 тестов)закрепляет вход свёртки тремя независимыми слоями:
Xобязана нести домен-сепаратор,который произносит
X.детерминированном входе, закреплённые по имени координаты.
форме, не содержащей байтов допущенных исходников. Правка
interval.cgolden не двигает, поэтому регенерация «ради зелёного CI» невозможна:
любое его движение семантическое.
Оракул разбора написан от формата провода, а не вызовом кодировщика
pipeline:иначе закон именования и golden доказывали бы сами себя.
Дефект и замер
Класс был реально открыт для пары
engine_release/upstream_source.Мутация «поменять эти две координаты местами» до среза не ловилась ничем.
Замер (прогон всех 210 тестов каталога
arb/tests, мутация вarb/pipeline.py):engine_release/upstream_sourcewrapper_source/evaluator_sourcestdout/stderrвbuild_process_bytes_v1Вторая строка — почему в докстроке названа именно первая пара: перестановку
wrapper/evaluatorуже ловил предсуществующийtest_pipeline.ComparatorDerivationTests.test_wrapper_and_evaluator_file_sets_are_exact_and_disjoint.Первая пара не ловилась ничем.
Что доказано
отсутствующий сепаратор.
exit=0) в обоих режимах, включаяPYTHONOPTIMIZE=2,где
assert-ы исчезают:arb/tests/gate.py— 292 теста, inventory3976521d…mpfi/tests/gate.py— 29 тестовЧто НЕ доказано (прямо)
build_identity,test_observation)этим срезом не закреплено. Выживший мутант: перестановка
stdout/stderrвнутри
build_process_bytes_v1оставляет все 210 тестовarb/testsзелёными. Байты процессов лежат в чанках длиннее 64 Б, а структурный слой
закрепляет такой чанк только присутствием и позицией — не длиной и не
байтами. Это граница слоя, а не покрытие, и так и записано в докстроке.
Отдельно проверено, что эту мутацию видит внешний golden
tests/test_build.py::test_build_process_encoding_is_total_and_keeps_exact_golden:фактический дайджест сдвигается
23e6682d…→8f846d19….proof/region/v1/testsдаёт 3 отказа, и онипредсуществующие: тот же набор с теми же дайджестами воспроизводится на
чистом
origin/main(515 тестов, 3 отказа — до и после среза одинаково).Причина — версия Python: CI пинует 3.14.6 в контейнере, локальная
проверка шла на 3.12.3. Срез не добавляет ни одного отказа, но и не
чинит эти три.
Пины инвентаря
Пины живут в двух местах и конфликтовали по построению — обе стороны устарели
(ветка добавила 8 тестов, main добавил свои). Не взята ни одна сторона:
объединение пересчитано исполнением энумерации гейта — 292 теста
(284 с main + 8 отсюда).
arb/tests/gate.py→EXPECTED_TEST_INVENTORY_SHA256tests/test_build.py→ARB_INVENTORY_SHA256_V1,ARB_ORDER_SHA256_V1,ARB_TEST_COUNT_V1(намеренно независимый внешний оракул — пересчитан, неимпортирован из гейта)
Слито два тика main:
#554, затем#555(main уехал во время работы).После
#555пины перепроверены исполнением — не сдвинулись.Откат
Срез добавляет один тестовый модуль и трогает только значения пинов.
Продуктового кода не меняет.
Либо точечно: удалить
proof/region/v1/arb/tests/test_comparator_content.pyивернуть пины на значения
#555(3284dccf…/3625426e…/284) — гейтснова сойдётся.
Гейт, который НЕ закрыт
Независимое review финального состояния не выполнено. Это «реализация
завершена», а не «готово».
Summary by CodeRabbit