Core: ввести типизированные направленные отношения - #489
Conversation
WalkthroughДобавлены directed relation constraints и intrinsic-unary проверки, специализированные Program Session body/evidence и typed compile errors. Content identity переведён на V6, evaluation arena расширена relation members, а тесты, source inventories и контрольные хэши обновлены. ChangesRelation constraints и Program Session
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview 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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/labcolors-core/src/generic_boundary_tests.rs (1)
1312-1322: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueЗапрет
ProgramSessionEvaluationError::VisibleUnaryне защищает ни от чего.Остальные записи в этом forbidden-списке — реальные символы прежнего eager-LCS пути. У
ProgramSessionEvaluationErrorвариантаVisibleUnaryне существует ни в старом, ни в новом коде, поэтому assert выполняется тривиально. Если цель — запретить возврат unary-специфичной ошибки из evaluation-пути, стоит закрепить реально существующий анти-дрейф маркер; иначе строку лучше убрать, чтобы не создавать иллюзию покрытия.Проверьте, какие варианты действительно объявлены в enum:
#!/bin/bash ast-grep run --pattern $'pub enum ProgramSessionEvaluationError<$_> { $$$ }' --lang rust crates/labcolors-core/src/program_session.rs rg -nP 'ProgramSessionEvaluationError::\w+' crates/labcolors-core/src --no-heading | sort -u | head -50🤖 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-core/src/generic_boundary_tests.rs` around lines 1312 - 1322, Remove the nonexistent ProgramSessionEvaluationError::VisibleUnary entry from the forbidden list in the generic boundary test. Keep the other eager-LCS markers unchanged, unless inspection of the declared ProgramSessionEvaluationError variants identifies an existing unary-specific evaluation error that should replace it.crates/labcolors-core/src/program_identity_tests.rs (1)
768-795: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnti-drift gate покрывает диапазоны, но не «полноту» словаря.
assert_eq!(schema.0, 1..=21)иassert_eq!(schema.1, 1..=27)фиксируют, что fixture исполняет каждый tag/role. Однако при добавлении нового варианта вEdgeRoleV1тест упадёт лишь на верхней границе, если варианты остаются непрерывными; при добавлении варианта в середину с последующим сдвигом golden-дайджест поймает изменение, но сообщение об ошибке будет неинформативным. Стоит сопоставлять со счётчиком вариантов изprogram_identity(например, экспортироватьEDGE_ROLE_COUNT_V1), чтобы падение сразу указывало на пропущенный role.🤖 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-core/src/program_identity_tests.rs` around lines 768 - 795, Update complete_program_schema_v6_digest_is_cross_platform_golden to validate the fixture’s edge-role coverage against an exported EdgeRoleV1 variant count from program_identity, such as EDGE_ROLE_COUNT_V1, rather than relying only on the hard-coded 1..=27 range. Preserve the existing schema and digest assertions while making missing or newly added roles produce an explicit count-based failure.
🤖 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/program_identity.rs`:
- Around line 660-666: Rename the ConstraintGraphBindingContextV1.scenario_group
field and its associated EdgeRoleV1::ScenarioGroup usage to clearly identify the
bound observation-group vertex, updating build_graph and all consumers
consistently; do not leave scenario terminology that could imply a separate
scenario vertex.
- Around line 677-702: In the VisibleUnary arm of the match, replace the
explicit occurrence binding and let _ discard with a .. pattern, while retaining
the invocation binding and existing constraint_color call unchanged.
- Around line 1022-1029: Replace the unchecked color.as_slice()[0] access in the
vertex_tags construction with typed handling for empty colors: return
ProgramCompileError::InternalInvariant when a color has no bytes, while
preserving tag collection, sorting, and deduplication for non-empty colors.
In `@crates/labcolors-core/src/program_mixed_evaluator_tests.rs`:
- Around line 836-845: Update the checksum handling in the
ConstraintSubjectV1::IntrinsicRelation and ConstraintSubjectV1::VisibleRelation
branches to incorporate their candidates data as well as reference, so differing
candidate sets produce differing checksums. Also address the relation fixture
path around consume_public_assessment so AssessmentV1::Relation is no longer an
unhandled panic, or explicitly mark these branches as unsupported stubs until a
relation fixture exists.
In `@crates/labcolors-core/src/program_relation_tests.rs`:
- Around line 28-44: Обновите тест
exact_relation_draft_api_requires_an_explicit_physical_level: либо добавьте
assertions, проверяющие состав и физический уровень отношений в draft после
вызовов push_exact_intrinsic_relation_hard и push_exact_visible_relation_hard,
либо переименуйте тест так, чтобы он явно описывал проверяемую type-level
гарантию через TargetIdV1 и OccurrenceIdV1.
- Around line 409-453: Разделите тест
relation_identity_ignores_opaque_names_and_candidate_declaration_order на
независимые проверки: сравните разные offset при одинаковом reverse_candidates
для инвариантности opaque-имён и одинаковый offset при разных reverse_candidates
для порядка candidates. Сохраните существующее сравнение комбинированного случая
только при необходимости, чтобы каждая ось отдельно выявляла регрессии.
In `@crates/labcolors-core/src/program_session.rs`:
- Around line 4126-4296: Extract the shared relation-arena accumulation logic
from the IntrinsicRelation and VisibleRelation branches into a helper near the
surrounding evaluation code, parameterized by candidate count and a closure that
resolves each candidate and returns its evidence plus violation status. Move
capture_start, capacity preflight, evidence accumulation, violation tracking,
NonEmptyRelationMemberSpanV1 construction, and Pass/Violation selection into
that helper; keep each branch responsible only for endpoint resolution,
evidence-member construction, and subject creation.
- Around line 5532-5551: Update the target scan in the constraint-processing
flow around dependency_scratch.scan to validate that dependency_scratch.targets
has the expected length before indexing it alongside program.targets. On
mismatch, return the existing typed InternalInvariant error used by
merge_visible_constraint_coverage; otherwise preserve the current finite-target
detection and SolverDependentVisibleRelationReference behavior.
- Around line 4001-4003: Move the invariant check comparing
constraint.scenario_group with epoch.observation_group.id out of the per-case ×
constraint hot loop and perform it once before iterating cases, validating all
constraints there. Remove the repeated check from the loop while preserving the
existing InternalInvariant error behavior and direct-index execution path.
In `@crates/labcolors-core/src/program.rs`:
- Around line 561-567: Добавьте `///`-документацию ко всем семи новым вариантам
`CompileErrorKindV1` и к каждому новому полю `CompileErrorV1` в указанном
диапазоне, сохранив стиль соседних вариантов и полей. Для
`SolverDependentVisibleRelationReference` явно укажите, что `target` —
finite-цель, из-за которой reference стал solver-зависимым.
- Around line 2462-2477: Уберите оба вызова unreachable! в преобразовании
ProgramConstraintResultV1 в AssessmentV1: relation_members_for может вернуть
None, поэтому проекция не должна паниковать. Измените AssessmentV1 или ближайший
flow построения RelationEvidenceV1 так, чтобы недоступное relation evidence
возвращалось типизированно (предпочтительно передавайте срез при построении
ячейки), сохранив корректные verdict для Pass и Violation.
- Around line 2680-2728: Измените IntrinsicRelationMemberV1 и
VisibleRelationMemberV1 так, чтобы они хранили распакованные ссылки на
соответствующие binding/endpoint и verdict, а не весь
ProgramRelationMemberEvidenceV1. При создании обёрток распакуйте вариант один
раз и передайте эти значения в поля; accessor'ы reference, candidate и verdict
должны возвращать сохранённые значения без повторной проверки и unreachable!().
Сохраните существующее поведение для корректных вариантов.
- Around line 1521-1548: Remove the panic-producing expect calls from
push_exact_intrinsic_relation_hard and push_exact_visible_relation_hard. Prefer
propagating try_map failures as a typed DraftErrorV1 by changing these facade
methods to return Result<&mut Self, DraftErrorV1> and mapping
DirectedRelationErrorV1 accordingly; update their callers to handle the Result
without introducing fallback behavior.
---
Outside diff comments:
In `@crates/labcolors-core/src/generic_boundary_tests.rs`:
- Around line 1312-1322: Remove the nonexistent
ProgramSessionEvaluationError::VisibleUnary entry from the forbidden list in the
generic boundary test. Keep the other eager-LCS markers unchanged, unless
inspection of the declared ProgramSessionEvaluationError variants identifies an
existing unary-specific evaluation error that should replace it.
In `@crates/labcolors-core/src/program_identity_tests.rs`:
- Around line 768-795: Update
complete_program_schema_v6_digest_is_cross_platform_golden to validate the
fixture’s edge-role coverage against an exported EdgeRoleV1 variant count from
program_identity, such as EDGE_ROLE_COUNT_V1, rather than relying only on the
hard-coded 1..=27 range. Preserve the existing schema and digest assertions
while making missing or newly added roles produce an explicit count-based
failure.
🪄 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: 05d1e967-c9bb-42f3-9e23-f491b7c88758
📒 Files selected for processing (25)
crates/labcolors-core/contracts/clean-set-srgb8-v1/receipt-v1.jsoncrates/labcolors-core/contracts/clean-set-srgb8-v1/receipt-v1.sha256crates/labcolors-core/contracts/point-support-reference-surplus-q55-bps-proof-v1.jsoncrates/labcolors-core/src/appearance.rscrates/labcolors-core/src/constraints/mod.rscrates/labcolors-core/src/constraints/relation.rscrates/labcolors-core/src/generic_boundary_tests.rscrates/labcolors-core/src/lib.rscrates/labcolors-core/src/observation.rscrates/labcolors-core/src/program.rscrates/labcolors-core/src/program/attachment/tests.rscrates/labcolors-core/src/program_api_tests.rscrates/labcolors-core/src/program_boundary_tests.rscrates/labcolors-core/src/program_clean_set_tests.rscrates/labcolors-core/src/program_identity.rscrates/labcolors-core/src/program_identity_tests.rscrates/labcolors-core/src/program_joint_integration_tests.rscrates/labcolors-core/src/program_lcs_integration_tests.rscrates/labcolors-core/src/program_mixed_evaluator_tests.rscrates/labcolors-core/src/program_point_causality_tests.rscrates/labcolors-core/src/program_relation_tests.rscrates/labcolors-core/src/program_session.rscrates/labcolors-core/src/program_session_tests.rscrates/labcolors-core/src/relation.rsscripts/verify_point_support_surplus.py
| let graph = build_graph(program)?; | ||
| let mut vertex_tags = Vec::new(); | ||
| vertex_tags | ||
| .try_reserve_exact(graph.colors.len()) | ||
| .map_err(|_| ProgramCompileError::ResourceExhausted)?; | ||
| vertex_tags.extend(graph.colors.iter().map(|color| color.as_slice()[0])); | ||
| vertex_tags.sort_unstable(); | ||
| vertex_tags.dedup(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Индексация as_slice()[0] — единственный panic-путь в новом helper.
Все остальные шаги функции возвращают типизированную ошибку (ResourceExhausted), а извлечение tag полагается на непустоту цвета. Инвариант «VertexColorV1::new всегда пишет tag первым байтом» здесь не выражен: либо зафиксируйте его комментарием «почему», либо верните ProgramCompileError::InternalInvariant для пустого цвета.
🛡️ Вариант с типизированным отказом
- vertex_tags.extend(graph.colors.iter().map(|color| color.as_slice()[0]));
+ for color in &graph.colors {
+ // Первый байт цвета — vertex tag; пустой цвет означает нарушенный
+ // инвариант writer'а, а не допустимую схему.
+ let [tag, ..] = color.as_slice() else {
+ return Err(ProgramCompileError::InternalInvariant);
+ };
+ vertex_tags.push(*tag);
+ }Согласно coding guidelines: «Новый или изменяемый public path не должен вызывать panic … invalid, unreachable, unsupported и incomplete context должны возвращаться типизированно».
📝 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.
| let graph = build_graph(program)?; | |
| let mut vertex_tags = Vec::new(); | |
| vertex_tags | |
| .try_reserve_exact(graph.colors.len()) | |
| .map_err(|_| ProgramCompileError::ResourceExhausted)?; | |
| vertex_tags.extend(graph.colors.iter().map(|color| color.as_slice()[0])); | |
| vertex_tags.sort_unstable(); | |
| vertex_tags.dedup(); | |
| let graph = build_graph(program)?; | |
| let mut vertex_tags = Vec::new(); | |
| vertex_tags | |
| .try_reserve_exact(graph.colors.len()) | |
| .map_err(|_| ProgramCompileError::ResourceExhausted)?; | |
| for color in &graph.colors { | |
| // Первый байт цвета — vertex tag; пустой цвет означает нарушенный | |
| // инвариант writer'а, а не допустимую схему. | |
| let [tag, ..] = color.as_slice() else { | |
| return Err(ProgramCompileError::InternalInvariant); | |
| }; | |
| vertex_tags.push(*tag); | |
| } | |
| vertex_tags.sort_unstable(); | |
| vertex_tags.dedup(); |
🤖 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-core/src/program_identity.rs` around lines 1022 - 1029,
Replace the unchecked color.as_slice()[0] access in the vertex_tags construction
with typed handling for empty colors: return
ProgramCompileError::InternalInvariant when a color has no bytes, while
preserving tag collection, sorting, and deduplication for non-empty colors.
Source: Coding guidelines
| ConstraintSubjectV1::IntrinsicUnary { target } => { | ||
| probe.mix(u64::from(target.value())); | ||
| } | ||
| ConstraintSubjectV1::IntrinsicRelation { reference } => { | ||
| probe.mix(u64::from(reference.value())); | ||
| } | ||
| ConstraintSubjectV1::VisibleRelation { reference, .. } => { | ||
| probe.mix(u64::from(reference.value())); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Checksum relation-веток игнорирует candidates.
IntrinsicRelation/VisibleRelation подмешивают только reference, поэтому два отношения с разными наборами candidates дадут одинаковый checksum. Одновременно consume_public_assessment (строки 689–691) паникует на AssessmentV1::Relation, то есть ветки в текущей фикстуре недостижимы и не проверены. Либо подмешайте candidates, либо явно зафиксируйте, что ветки — заглушки до появления relation-фикстуры.
♻️ Пример полного подмешивания
- ConstraintSubjectV1::VisibleRelation { reference, .. } => {
- probe.mix(u64::from(reference.value()));
- }
+ ConstraintSubjectV1::VisibleRelation {
+ reference,
+ candidates,
+ } => {
+ probe.mix(u64::from(reference.value()));
+ for candidate in candidates {
+ probe.mix(u64::from(candidate.value()));
+ }
+ }#!/bin/bash
# Проверяем реальную форму вариантов ConstraintSubjectV1 (наличие поля candidates)
fd -t f 'program' crates/labcolors-core/src --exec ast-grep run --lang rust --pattern 'enum ConstraintSubjectV1 { $$$ }' {} \;
rg -nP -C6 'VisibleRelation\s*\{' crates/labcolors-core/src --type=rustAlso applies to: 880-889
🤖 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-core/src/program_mixed_evaluator_tests.rs` around lines 836
- 845, Update the checksum handling in the
ConstraintSubjectV1::IntrinsicRelation and ConstraintSubjectV1::VisibleRelation
branches to incorporate their candidates data as well as reference, so differing
candidate sets produce differing checksums. Also address the relation fixture
path around consume_public_assessment so AssessmentV1::Relation is no longer an
unhandled panic, or explicitly mark these branches as unsupported stubs until a
relation fixture exists.
| pub(crate) fn push_exact_intrinsic_relation_hard( | ||
| &mut self, | ||
| id: ConstraintIdV1, | ||
| relation: DirectedRelationV1<TargetIdV1>, | ||
| ) -> &mut Self { | ||
| self.inner.push_exact_intrinsic_relation_hard( | ||
| id.into_core(), | ||
| relation | ||
| .try_map(TargetIdV1::into_core) | ||
| .expect("facade TargetId is a transparent bijection over Core TargetId"), | ||
| ); | ||
| self | ||
| } | ||
|
|
||
| /// Добавляет обязательное exact-отношение между final modeled Occurrences. | ||
| pub(crate) fn push_exact_visible_relation_hard( | ||
| &mut self, | ||
| id: ConstraintIdV1, | ||
| relation: DirectedRelationV1<OccurrenceIdV1>, | ||
| ) -> &mut Self { | ||
| self.inner.push_exact_visible_relation_hard( | ||
| id.into_core(), | ||
| relation | ||
| .try_map(OccurrenceIdV1::into_core) | ||
| .expect("facade OccurrenceId is a transparent bijection over Core OccurrenceId"), | ||
| ); | ||
| self | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
expect на facade-пути Draft противоречит и guideline, и собственному инварианту try_map.
try_map специально спроектирован так, чтобы «даже внутреннему mapper нельзя доверять по комментарию» (relation.rs, строки 64‑67) — и возвращает типизированную DirectedRelationErrorV1. Здесь эта типизированная ошибка немедленно сворачивается в panic на публичном пути построения Draft.
Два варианта без panic:
- Хранить в facade
DirectedRelationV1<TargetId>/DirectedRelationV1<OccurrenceId>уже в Core-представлении, чтобы remap на этом уровне вообще не требовался; - Либо вернуть
Result<&mut Self, DraftErrorV1>и смапитьDirectedRelationErrorV1в типизированный вариант.
♻️ Набросок варианта 2
pub(crate) fn push_exact_intrinsic_relation_hard(
&mut self,
id: ConstraintIdV1,
relation: DirectedRelationV1<TargetIdV1>,
- ) -> &mut Self {
- self.inner.push_exact_intrinsic_relation_hard(
- id.into_core(),
- relation
- .try_map(TargetIdV1::into_core)
- .expect("facade TargetId is a transparent bijection over Core TargetId"),
- );
- self
+ ) -> Result<&mut Self, DraftErrorV1> {
+ let relation = relation
+ .try_map(TargetIdV1::into_core)
+ .map_err(|_| DraftErrorV1::InvalidRelationTopology)?;
+ self.inner
+ .push_exact_intrinsic_relation_hard(id.into_core(), relation);
+ Ok(self)
}Как per coding guidelines: «Новый или изменяемый public path не должен вызывать panic и не должен получать plausible fallback; invalid, unreachable, unsupported и incomplete context должны возвращаться типизированно».
🤖 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-core/src/program.rs` around lines 1521 - 1548, Remove the
panic-producing expect calls from push_exact_intrinsic_relation_hard and
push_exact_visible_relation_hard. Prefer propagating try_map failures as a typed
DraftErrorV1 by changing these facade methods to return Result<&mut Self,
DraftErrorV1> and mapping DirectedRelationErrorV1 accordingly; update their
callers to handle the Result without introducing fallback behavior.
Source: Coding guidelines
| ProgramConstraintResultV1::Pass(ProgramConstraintPassEvidenceV1::Relation(span)) => { | ||
| AssessmentV1::Relation(RelationEvidenceV1 { | ||
| verdict: VerdictV1::Pass, | ||
| members: report | ||
| .relation_members_for(*span) | ||
| .unwrap_or_else(|| unreachable!("report owns every relation span")), | ||
| }) | ||
| } | ||
| ProgramConstraintResultV1::Violation(ProgramConstraintViolationEvidenceV1::Relation( | ||
| span, | ||
| )) => AssessmentV1::Relation(RelationEvidenceV1 { | ||
| verdict: VerdictV1::Violation, | ||
| members: report | ||
| .relation_members_for(*span) | ||
| .unwrap_or_else(|| unreachable!("report owns every relation span")), | ||
| }), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
unreachable! при разрешении relation-span — panic на пути проекции evidence.
Тип не связывает span с конкретным report: NonEmptyRelationMemberSpanV1 — обычная пара start/len, а relation_members_for возвращает Option. Здесь это разворачивается в panic вместо типизированного результата.
Минимальный вариант без panic и без изменения публичной формы — добавить в AssessmentV1 явный вариант «evidence недоступно», либо (предпочтительно) сделать span непроецируемым наружу, отдавая срез прямо в момент построения ячейки. Как per coding guidelines: «invalid, unreachable, unsupported и incomplete context должны возвращаться типизированно».
🤖 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-core/src/program.rs` around lines 2462 - 2477, Уберите оба
вызова unreachable! в преобразовании ProgramConstraintResultV1 в AssessmentV1:
relation_members_for может вернуть None, поэтому проекция не должна паниковать.
Измените AssessmentV1 или ближайший flow построения RelationEvidenceV1 так,
чтобы недоступное relation evidence возвращалось типизированно (предпочтительно
передавайте срез при построении ячейки), сохранив корректные verdict для Pass и
Violation.
Source: Coding guidelines
| #[derive(Clone, Copy)] | ||
| pub(crate) struct IntrinsicRelationMemberV1<'a> { | ||
| inner: &'a ProgramRelationMemberEvidenceV1, | ||
| } | ||
|
|
||
| impl IntrinsicRelationMemberV1<'_> { | ||
| pub(crate) const fn reference(self) -> IntrinsicPaintBindingV1 { | ||
| let Some((reference, _)) = self.inner.intrinsic_bindings() else { | ||
| unreachable!() | ||
| }; | ||
| IntrinsicPaintBindingV1::from_core(*reference) | ||
| } | ||
|
|
||
| pub(crate) const fn candidate(self) -> IntrinsicPaintBindingV1 { | ||
| let Some((_, candidate)) = self.inner.intrinsic_bindings() else { | ||
| unreachable!() | ||
| }; | ||
| IntrinsicPaintBindingV1::from_core(*candidate) | ||
| } | ||
|
|
||
| pub(crate) const fn verdict(self) -> VerdictV1 { | ||
| project_relation_verdict(self.inner.decision()) | ||
| } | ||
| } | ||
|
|
||
| #[derive(Clone, Copy)] | ||
| pub(crate) struct VisibleRelationMemberV1<'a> { | ||
| inner: &'a ProgramRelationMemberEvidenceV1, | ||
| } | ||
|
|
||
| impl<'a> VisibleRelationMemberV1<'a> { | ||
| pub(crate) const fn reference(self) -> VisibleRelationEndpointV1<'a> { | ||
| let Some((reference, _)) = self.inner.visible_bindings() else { | ||
| unreachable!() | ||
| }; | ||
| VisibleRelationEndpointV1 { inner: reference } | ||
| } | ||
|
|
||
| pub(crate) const fn candidate(self) -> VisibleRelationEndpointV1<'a> { | ||
| let Some((_, candidate)) = self.inner.visible_bindings() else { | ||
| unreachable!() | ||
| }; | ||
| VisibleRelationEndpointV1 { inner: candidate } | ||
| } | ||
|
|
||
| pub(crate) const fn verdict(self) -> VerdictV1 { | ||
| project_relation_verdict(self.inner.decision()) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Голые unreachable!() в accessor'ах member-evidence лучше устранить структурно.
IntrinsicRelationMemberV1 / VisibleRelationMemberV1 хранят весь ProgramRelationMemberEvidenceV1 и заново проверяют вариант в каждом accessor. Поскольку from_core уже разобрал вариант, достаточно сохранить в обёртке сразу распакованные ссылки — тогда panic-ветки исчезают вместе с необходимостью в unreachable!().
♻️ Предлагаемая форма
#[derive(Clone, Copy)]
pub(crate) struct IntrinsicRelationMemberV1<'a> {
- inner: &'a ProgramRelationMemberEvidenceV1,
+ reference: &'a ProgramIntrinsicPaintBindingV1,
+ candidate: &'a ProgramIntrinsicPaintBindingV1,
+ decision: ProgramRelationMemberDecisionV1,
}
impl IntrinsicRelationMemberV1<'_> {
pub(crate) const fn reference(self) -> IntrinsicPaintBindingV1 {
- let Some((reference, _)) = self.inner.intrinsic_bindings() else {
- unreachable!()
- };
- IntrinsicPaintBindingV1::from_core(*reference)
+ IntrinsicPaintBindingV1::from_core(*self.reference)
}Как per coding guidelines: «Новый или изменяемый public path не должен вызывать panic».
🤖 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-core/src/program.rs` around lines 2680 - 2728, Измените
IntrinsicRelationMemberV1 и VisibleRelationMemberV1 так, чтобы они хранили
распакованные ссылки на соответствующие binding/endpoint и verdict, а не весь
ProgramRelationMemberEvidenceV1. При создании обёрток распакуйте вариант один
раз и передайте эти значения в поля; accessor'ы reference, candidate и verdict
должны возвращать сохранённые значения без повторной проверки и unreachable!().
Сохраните существующее поведение для корректных вариантов.
Source: Coding guidelines
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
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)
crates/labcolors-core/src/generic_boundary_tests.rs (1)
1464-1475: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winОжидание для
VisibleRelationслабее, чем заявленное сообщение теста.Для
VisibleUnaryпроверяется и извлечениеoccurrence, и запись индекса, а дляVisibleRelation— только факт наличия имени варианта. Сообщение утверждает «remap every constraint», но удаление обхода candidate-endpoint'ов внутри веткиVisibleRelationэтот gate не поймает: строка"CompiledProgramConstraintBodyV1::VisibleRelation"останется. Стоит зафиксировать обход reference и candidates явно.♻️ Предлагаемое усиление
- "CompiledProgramConstraintBodyV1::VisibleRelation", + "CompiledProgramConstraintBodyV1::VisibleRelation { reference, candidates, .. }", + "for endpoint in core::iter::once(reference).chain(candidates.iter_mut())",Точные литералы подберите под фактическую форму
compact_constraint_contexts.🤖 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-core/src/generic_boundary_tests.rs` around lines 1464 - 1475, Усильте проверки в тесте вокруг compact_constraint_contexts: вместо проверки только наличия варианта CompiledProgramConstraintBodyV1::VisibleRelation зафиксируйте отдельными уникальными литералами обход reference и candidate endpoints, включая извлечение occurrence и запись remapped index. Подберите строки по фактической реализации, чтобы тест обнаруживал удаление этого обхода и действительно проверял remap каждой constraint.
🤖 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/program_joint_integration_tests.rs`:
- Around line 2335-2358: Расширьте тест вокруг финальной проверки
`session.commit(update(1, 0x00))`, добавив constraint с `report_only`, который
возвращает `Violation`. Сохраните ожидаемый `FinalRecheckViolation` и убедитесь,
что его `hard_violation_count` остаётся равным 2, подтверждая подсчёт только
hard-нарушений.
In `@crates/labcolors-core/src/program_relation_tests.rs`:
- Around line 603-608: Разведите числовые идентификаторы в тесте вокруг
`third_candidate` и соответствующего `ConstraintIdV1`: замените переиспользуемое
значение `14` для одной из сущностей на другое уникальное значение, сохранив
корректность проверок координат по индексам и остальную логику теста.
---
Outside diff comments:
In `@crates/labcolors-core/src/generic_boundary_tests.rs`:
- Around line 1464-1475: Усильте проверки в тесте вокруг
compact_constraint_contexts: вместо проверки только наличия варианта
CompiledProgramConstraintBodyV1::VisibleRelation зафиксируйте отдельными
уникальными литералами обход reference и candidate endpoints, включая извлечение
occurrence и запись remapped index. Подберите строки по фактической реализации,
чтобы тест обнаруживал удаление этого обхода и действительно проверял remap
каждой constraint.
🪄 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: f539d4ab-e942-45c1-aea8-fd5344285b21
📒 Files selected for processing (10)
crates/labcolors-core/contracts/clean-set-srgb8-v1/receipt-v1.jsoncrates/labcolors-core/contracts/clean-set-srgb8-v1/receipt-v1.sha256crates/labcolors-core/src/generic_boundary_tests.rscrates/labcolors-core/src/program.rscrates/labcolors-core/src/program_identity.rscrates/labcolors-core/src/program_identity_tests.rscrates/labcolors-core/src/program_joint_integration_tests.rscrates/labcolors-core/src/program_point_causality_tests.rscrates/labcolors-core/src/program_relation_tests.rscrates/labcolors-core/src/program_session.rs
| vec![], | ||
| evaluator, | ||
| ) | ||
| .with_joint_selection(DeclaredJointSelectionV1::new(vec![state(FIRST)])) | ||
| .compile() | ||
| .unwrap(); | ||
| let mut session = compiled.instantiate(STREAM).unwrap(); | ||
|
|
||
| let error = match session.commit(update(1, 0x00)) { | ||
| Ok(_) => panic!("two failures in the fresh recheck must reject the selected state"), | ||
| Err(error) => error, | ||
| }; | ||
| assert_eq!( | ||
| error, | ||
| SessionUpdateError::Plan(ProgramSessionEvaluationError::FinalRecheckViolation { | ||
| state_index: 0, | ||
| case_index: 0, | ||
| constraint: first_violation, | ||
| subject: ProgramConstraintSubjectV1::VisibleUnary { | ||
| occurrence: OCCURRENCE, | ||
| context: appearance_context(), | ||
| }, | ||
| hard_violation_count: 2, | ||
| }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Проверьте исключение report-only violation из итогового счётчика.
report_only пуст, поэтому тест подтверждает только подсчёт двух hard-нарушений. Добавьте report-only constraint, возвращающий Violation, и проверьте, что hard_violation_count остаётся 2. Иначе название теста не покрывает контракт «только hard violations».
🤖 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-core/src/program_joint_integration_tests.rs` around lines
2335 - 2358, Расширьте тест вокруг финальной проверки `session.commit(update(1,
0x00))`, добавив constraint с `report_only`, который возвращает `Violation`.
Сохраните ожидаемый `FinalRecheckViolation` и убедитесь, что его
`hard_violation_count` остаётся равным 2, подтверждая подсчёт только
hard-нарушений.
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Зачем
V5a заменяет специальные парные/ролевые ветви одной типизированной алгеброй физических отношений. Core видит направление и физический уровень связи, но не клиентскую семантику имён.
Что изменено
DirectedRelationV1<T>: reference и непустое уникальное множество candidates;constraint × case × candidate, без last-writer-wins;Границы
PR не вводит family generator, selection policy, sentiment, cleanliness auto или публичный Program API. Это общий физический фундамент для V5b/V5c.
Доказательства exact head
Head:
d2a2dfe0716fc9f448c0c69818e85e19181a65eb.8924bb2..d2a2dfe: No actionable comments, exact status SUCCESS.Merge допускается: exact head, CI, mutation и оба review-контура зелёные.