Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughДобавлен отдельный MPFI source lock с digest-only integrity policy, каноническим source closure и typed admission. Реализованы публичные конструкторы, fail-closed проверки, документация протокольных ограничений и unit-тесты. ChangesMPFI source lock
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant mpfi_source_lock_v1
participant admit_mpfi_sources
participant AdmittedMpfiSourcesV1
mpfi_source_lock_v1->>admit_mpfi_sources: ожидаемый MpfiSourceLockV1
admit_mpfi_sources->>AdmittedMpfiSourcesV1: проверенный source closure
AdmittedMpfiSourcesV1-->>admit_mpfi_sources: MPFI capability identity
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
602ff4c to
cd9e590
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
lemone112
changed the title
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 47 seconds. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![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
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/provenance.py (1)
625-646: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winДублирование валидации ролей/integrity между
ArbSourceLockV1.__post_init__иMpfiSourceLockV1.__post_init__.Обе реализации почти идентичны (тип/длина tuple, роль-последовательность, integrity-типы), различаясь только ожидаемыми ролями и integrity-классами. Учитывая, что в этом же PR уже вынесены общие
_encode_source_closure_v1/_parse_source_closure_v1, логично продолжить и вынести общий валидатор, параметризованный ожидаемыми ролями и integrity-типами — особенно если в будущем появятся другие independent-proof lanes.♻️ Пример общего валидатора
def _validate_source_closure_roles_v1( artifact: str, sources: _SourceClosureV1, expected_roles: tuple[SourceRoleV1, ...], expected_integrity: tuple[type[SourceIntegrityPolicyV1], ...], ) -> None: if type(sources) is not tuple or len(sources) != SOURCE_CLOSURE_COUNT_V1: _fail(artifact, ProvenanceReasonV1.INVALID_FIELD, "source count") if any(type(value) is not SourceReleaseLockV1 for value in sources): _fail(artifact, ProvenanceReasonV1.INVALID_FIELD, "source type") if tuple(value.role for value in sources) != expected_roles: _fail(artifact, ProvenanceReasonV1.NONCANONICAL_ORDER, ", ".join(r.name for r in expected_roles)) if any(type(value.integrity) is not kind for value, kind in zip(sources, expected_integrity, strict=True)): _fail(artifact, ProvenanceReasonV1.INTEGRITY_KIND_MISMATCH, "integrity policy")Also applies to: 668-693
🤖 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/provenance.py` around lines 625 - 646, Extract the duplicated source-closure validation from ArbSourceLockV1.__post_init__ and MpfiSourceLockV1.__post_init__ into a shared _validate_source_closure_roles_v1 helper. Parameterize it with the artifact name, sources, expected role tuple, and per-source integrity policy types, preserving the existing count, element-type, canonical-order, and integrity-mismatch failures and messages; replace both inline validation blocks with calls to the helper.
🤖 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/provenance.py`:
- Line 435: Remove the unnecessary quotes from the return annotations of
parse_from methods for ProjectPinnedArchiveDigestPolicyV1, ArbSourceLockV1, and
MpfiSourceLockV1 in provenance.py, using the existing from __future__ import
annotations support while preserving the referenced return types.
In `@proof/region/v1/tests/test_mpfi_source_lock.py`:
- Around line 220-221: Update the hostile source-order test around
admit_mpfi_sources to assert only ProvenanceErrorV1, then verify the exception’s
reason is ProvenanceReasonV1.FOREIGN_BINDING, matching the neighboring tests
instead of accepting TypeError.
---
Outside diff comments:
In `@proof/region/v1/provenance.py`:
- Around line 625-646: Extract the duplicated source-closure validation from
ArbSourceLockV1.__post_init__ and MpfiSourceLockV1.__post_init__ into a shared
_validate_source_closure_roles_v1 helper. Parameterize it with the artifact
name, sources, expected role tuple, and per-source integrity policy types,
preserving the existing count, element-type, canonical-order, and
integrity-mismatch failures and messages; replace both inline validation blocks
with calls to the helper.
🪄 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: 35a31e4d-0adf-4997-b16b-04a442076c32
📒 Files selected for processing (3)
proof/region/v1/PROTOCOL.mdproof/region/v1/provenance.pyproof/region/v1/tests/test_mpfi_source_lock.py
|
Проверил outside-diff предложение об общем |
|
@coderabbitai review Новый exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Exact commit diff ==='
git show --format=fuller --find-renames --find-copies --stat 443d0a8
git show --format= --find-renames --find-copies --unified=80 443d0a8 -- proof/region/v1/PROTOCOL.md
echo '=== Protocol structure ==='
ast-grep outline proof/region/v1/PROTOCOL.md --items all 2>/dev/null || true
rg -n -i -C 5 'u64|u32|wire|source.lock|source lock|codec|length|frame' proof/region/v1/PROTOCOL.md
echo '=== Provenance module structure ==='
ast-grep outline proof/region/v1/provenance.py --items all --type class,function --match 'encode|parse|Lock|Reader|Writer|closure|proof' || true
echo '=== Width-related implementation references ==='
rg -n -C 4 'u64|u32|write_u|read_u|pack\(|unpack\(|SourceLock|source_lock|proof wire|wire' proof/region/v1 -g '*.py'Length of output: 153057
Commit изменяет только документацию. Runtime codec не изменён. ✅ Action performedReview finished.
|
|
Superseded терминальным срезом #514 (Design Freeze M2a): весь объём этого PR — точный исходник MPFI и source-bound lane — полностью доставлен в #514 ( |
1 participant
Что изменено
ProjectPinnedArchiveDigestPolicyV1: archive digest фиксирует Lab Colors, без выдуманной upstream signature или Git relation;u64be-blob общего proof wire и отдельныйu32be-blob source-lock codecLCSRC1.Почему
Это первый самостоятельный срез MPFI proof path. Он допускает exact owned source bytes, но не повышает project-observed SHA-256 до publisher-authenticated evidence. Последняя документационная правка устраняет ложное описание public wire, не меняя codec.
Stacked on #502; merge только после родительской цепочки.
Проверки
8a63b2321aabf11d97b0ca9977152cb6802840bf: shared suite 91 tests PASS normal + optimized, 1 declared native skip; Arb regression 155 tests PASS normal + optimized, exact 11 skips; independent hostile review PASS.443d0a8:test_mpfi_source_lock6/6 normal + optimized; static scope check confirms bothu64beproof wire andu32besource-lock codec;git diff --checkPASS.