V2a: совместный hard-feasible point selection - #362
Conversation
WalkthroughДобавлен приватный модуль совместной оценки lower/upper Paint-кандидатов. Он формирует hard-отчёты по уникальным сценариям, классифицирует feasible-кортежи, выбирает их по declared order и выполняет revision-bound recheck выбранного результата. ChangesСовместная оценка point-selection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant JointPointProgramV1
participant FullHardReportV1
participant DeclaredTotalOrderV1
participant SelectedJointTupleV1
Client->>JointPointProgramV1: передаёт candidates и observation
JointPointProgramV1-->>FullHardReportV1: формирует executions и constraint cells
FullHardReportV1->>FullHardReportV1: классифицирует feasible ordinals
Client->>DeclaredTotalOrderV1: передаёт порядок кандидатов
DeclaredTotalOrderV1->>SelectedJointTupleV1: выбирает feasible ordinal
SelectedJointTupleV1->>JointPointProgramV1: повторно оценивает выбранный кандидат
JointPointProgramV1-->>SelectedJointTupleV1: возвращает fresh executions и cells
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 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 `@crates/labcolors-core/src/joint.rs`:
- Around line 545-599: Согласуйте API `DeclaredTotalOrderV1` с тестами: измените
`new` на принимающий только `Vec<CandidateOrdinalV1>` и возвращающий `Self` без
валидации, а `select` на возвращающий `Result<SelectedJointTupleV1,
SelectionPolicyErrorV1>`. Перенесите проверку длины, дубликатов и полного
соответствия кандидатов в `select` либо вспомогательный `validate_against`;
замените оба `unreachable!` типизированными ошибками и обновите вызываемый код
под новый результат.
🪄 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: f11ab933-5aae-4302-8b76-10488b5d8692
📒 Files selected for processing (3)
crates/labcolors-core/src/joint.rscrates/labcolors-core/src/joint_tests.rscrates/labcolors-core/src/lib.rs
| pub(crate) fn select(self, policy: DeclaredTotalOrderV1) -> SelectedJointTupleV1 { | ||
| let ordinal = policy | ||
| .order | ||
| .iter() | ||
| .copied() | ||
| .find(|ordinal| self.feasible.binary_search(ordinal).is_ok()) | ||
| .unwrap_or_else(|| unreachable!("validated total order covers nonempty feasible set")); | ||
| let candidate = *self | ||
| .report | ||
| .candidates | ||
| .candidates() | ||
| .iter() | ||
| .find(|candidate| candidate.ordinal == ordinal) | ||
| .unwrap_or_else(|| unreachable!("validated ordinal belongs to candidate set")); | ||
| SelectedJointTupleV1 { | ||
| report: self.report, | ||
| policy, | ||
| candidate, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Полный client-declared tie-break. Он не участвует в measurement/report. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub(crate) struct DeclaredTotalOrderV1 { | ||
| order: Box<[CandidateOrdinalV1]>, | ||
| } | ||
|
|
||
| impl DeclaredTotalOrderV1 { | ||
| pub(crate) fn new( | ||
| candidates: &JointCandidateSetV1, | ||
| order: Vec<CandidateOrdinalV1>, | ||
| ) -> Result<Self, SelectionPolicyErrorV1> { | ||
| if order.len() != candidates.candidates.len() { | ||
| return Err(SelectionPolicyErrorV1::NotATotalOrder); | ||
| } | ||
| let mut canonical = order.clone(); | ||
| canonical.sort_unstable(); | ||
| for pair in canonical.windows(2) { | ||
| if pair[0] == pair[1] { | ||
| return Err(SelectionPolicyErrorV1::DuplicateOrdinal(pair[0])); | ||
| } | ||
| } | ||
| if canonical | ||
| .iter() | ||
| .copied() | ||
| .ne(candidates.candidates.iter().map(|candidate| candidate.ordinal)) | ||
| { | ||
| return Err(SelectionPolicyErrorV1::NotATotalOrder); | ||
| } | ||
| Ok(Self { | ||
| order: order.into_boxed_slice(), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
Контракт select / DeclaredTotalOrderV1::new расходится с тестами — модуль не скомпилируется.
В этой реализации:
DeclaredTotalOrderV1::new(candidates: &JointCandidateSetV1, order: Vec<…>) -> Result<…>принимает два аргумента и валидирует порядок при построении;select(self, policy) -> SelectedJointTupleV1возвращает значение напрямую и опирается наunreachable!.
Но joint_tests.rs использует другой контракт:
DeclaredTotalOrderV1::new(vec![...])— один аргумент (например, строки 121, 259, 355);feasible.select(...).unwrap()иassert_eq!(feasible.select(DeclaredTotalOrderV1::new(vec![])), Err(SelectionPolicyErrorV1::NotATotalOrder))(строки 125, 354-357), т.е.selectдолжен возвращатьResult<SelectedJointTupleV1, SelectionPolicyErrorV1>, аnew— не выполнять валидацию.
Эти два контракта несовместимы, тестовый модуль не соберётся. Нужно выбрать единый контракт. Перенос валидации в select дополнительно устраняет unreachable! в public path.
Как per coding guidelines: «Новый или изменяемый public path не должен вызывать panic … invalid, unreachable, unsupported и incomplete context должны возвращаться типизированно».
🐛 Вариант выравнивания под контракт тестов (валидация в `select`)
- pub(crate) fn select(self, policy: DeclaredTotalOrderV1) -> SelectedJointTupleV1 {
- let ordinal = policy
- .order
- .iter()
- .copied()
- .find(|ordinal| self.feasible.binary_search(ordinal).is_ok())
- .unwrap_or_else(|| unreachable!("validated total order covers nonempty feasible set"));
- let candidate = *self
- .report
- .candidates
- .candidates()
- .iter()
- .find(|candidate| candidate.ordinal == ordinal)
- .unwrap_or_else(|| unreachable!("validated ordinal belongs to candidate set"));
- SelectedJointTupleV1 {
- report: self.report,
- policy,
- candidate,
- }
- }
+ pub(crate) fn select(
+ self,
+ policy: DeclaredTotalOrderV1,
+ ) -> Result<SelectedJointTupleV1, SelectionPolicyErrorV1> {
+ policy.validate_against(self.candidate_set())?;
+ let ordinal = policy
+ .order
+ .iter()
+ .copied()
+ .find(|ordinal| self.feasible.binary_search(ordinal).is_ok())
+ .ok_or(SelectionPolicyErrorV1::NotATotalOrder)?;
+ let candidate = *self
+ .report
+ .candidates
+ .candidates()
+ .iter()
+ .find(|candidate| candidate.ordinal == ordinal)
+ .ok_or(SelectionPolicyErrorV1::NotATotalOrder)?;
+ Ok(SelectedJointTupleV1 {
+ report: self.report,
+ policy,
+ candidate,
+ })
+ }и соответственно DeclaredTotalOrderV1::new(order: Vec<CandidateOrdinalV1>) -> Self без валидации, с выносом текущей логики в validate_against.
🤖 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/joint.rs` around lines 545 - 599, Согласуйте API
`DeclaredTotalOrderV1` с тестами: измените `new` на принимающий только
`Vec<CandidateOrdinalV1>` и возвращающий `Self` без валидации, а `select` на
возвращающий `Result<SelectedJointTupleV1, SelectionPolicyErrorV1>`. Перенесите
проверку длины, дубликатов и полного соответствия кандидатов в `select` либо
вспомогательный `validate_against`; замените оба `unreachable!` типизированными
ошибками и обновите вызываемый код под новый результат.
Source: Coding guidelines
9d248c5 to
6751e62
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/labcolors-core/src/joint_tests.rs (1)
335-397: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winОтсутствует тест на
SelectionPolicyErrorV1::DuplicateOrdinal.Здесь проверяется только
NotATotalOrder(через несовпадение длины:vec![]иvec![0,0]на домене из одного candidate — оба случая падают на проверке длины раньше, чем на проверке дублей). ВеткаDuplicateOrdinal(когдаorder.len() == candidates.len(), но есть повтор — требует минимум 2 candidate) остаётся непокрытой, хотя это тот же изменяемый валидационный путь вDeclaredTotalOrderV1::new.✅ Предложение добавить кейс
assert_eq!( DeclaredTotalOrderV1::new( &domain, vec![CandidateOrdinalV1::new(0), CandidateOrdinalV1::new(0)], ), Err(SelectionPolicyErrorV1::NotATotalOrder) ); + + let two = candidates(vec![ + candidate(0, ([0; 3], 1.0), ([0; 3], 1.0)), + candidate(1, ([1; 3], 1.0), ([1; 3], 1.0)), + ]); + assert_eq!( + DeclaredTotalOrderV1::new( + &two, + vec![CandidateOrdinalV1::new(0), CandidateOrdinalV1::new(0)], + ), + Err(SelectionPolicyErrorV1::DuplicateOrdinal(CandidateOrdinalV1::new(0))) + ); }🤖 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/joint_tests.rs` around lines 335 - 397, Добавьте в invalid_domains_and_policies_fail_before_compositing отдельный тест для DeclaredTotalOrderV1::new с доменом минимум из двух кандидатов и order той же длины, но с повторяющимся CandidateOrdinalV1; проверьте, что результатом является SelectionPolicyErrorV1::DuplicateOrdinal, сохранив существующие проверки NotATotalOrder.crates/labcolors-core/src/joint.rs (2)
618-641: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winМагическая константа
1без комментария об инварианте.
checked_joint_cardinality(1, cases, self.report.program.constraints.len())— литерал1кодирует «recheck выполняется ровно для одного выбранного candidate», что неочевидно без контекста ниже (core::slice::from_ref(&self.candidate)). Согласно path instructions дляcrates/labcolors-core/**/*.rs: «Отсутствие магических констант без комментариев». Короткий комментарий, объясняющий инвариант (а не пересказ оператора), сделает этот участок нагляднее для будущих изменений.✏️ Предложение
let cases = self.report.observation.set().cases().len(); + // recheck затрагивает ровно один ранее выбранный candidate — + // поэтому cardinality считается для единичного кандидата. let (execution_count, cell_count) = checked_joint_cardinality(1, cases, self.report.program.constraints.len()) .map_err(|_| SelectedRecheckErrorV1::ResourceExhausted)?;🤖 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/joint.rs` around lines 618 - 641, Добавьте краткий комментарий непосредственно перед вызовом checked_joint_cardinality в методе recheck, поясняющий, что значение 1 соответствует инварианту выполнения recheck ровно для одного выбранного candidate, передаваемого через core::slice::from_ref(&self.candidate). Не изменяйте вычисления или остальную логику.Source: Path instructions
572-598: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
select()не должен опираться наunreachable!()
crates/labcolors-core/src/joint.rs:544-563
Сейчас обе fallback-ветки могут паниковать; для crate-visible API лучше протянутьResultи вернуть типизированную ошибку вместо 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/joint.rs` around lines 572 - 598, Update select() to return a Result instead of relying on unreachable!() in either fallback branch. Replace both panic paths with the appropriate typed SelectionPolicyErrorV1 value, propagate the result through callers, and preserve the existing successful selection behavior.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@crates/labcolors-core/src/joint_tests.rs`:
- Around line 335-397: Добавьте в
invalid_domains_and_policies_fail_before_compositing отдельный тест для
DeclaredTotalOrderV1::new с доменом минимум из двух кандидатов и order той же
длины, но с повторяющимся CandidateOrdinalV1; проверьте, что результатом
является SelectionPolicyErrorV1::DuplicateOrdinal, сохранив существующие
проверки NotATotalOrder.
In `@crates/labcolors-core/src/joint.rs`:
- Around line 618-641: Добавьте краткий комментарий непосредственно перед
вызовом checked_joint_cardinality в методе recheck, поясняющий, что значение 1
соответствует инварианту выполнения recheck ровно для одного выбранного
candidate, передаваемого через core::slice::from_ref(&self.candidate). Не
изменяйте вычисления или остальную логику.
- Around line 572-598: Update select() to return a Result instead of relying on
unreachable!() in either fallback branch. Replace both panic paths with the
appropriate typed SelectionPolicyErrorV1 value, propagate the result through
callers, and preserve the existing successful selection behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 34343a2a-f681-418e-be83-697b480ba74d
📒 Files selected for processing (2)
crates/labcolors-core/src/joint.rscrates/labcolors-core/src/joint_tests.rs
1 participant
Срез
Первый private V2a consumer после V1b/S0. Две Paint-переменные исполняются как одна code-owned physical topology:
Что реализовано
JointCandidateSetV1с уникальнымиCandidateOrdinalV1и fail-closed duplicate physical tuple admission;DeclaredTotalOrderV1, который не участвует в measurement/report;candidate × casejoint execution report иcandidate × constraint × casehard-report без short-circuit;Error = Infallible, отдельного fault-verdict нет;try_reserve_exactдо первого compositor/evaluator call;RevisionBoundVerifiedSelectionV1: terminal output certificate, Session, Pair и public Program не вводятся.RED/acceptance
Границы
a32491d7.6751e624; changed files:joint.rs,joint_tests.rs,lib.rs.labcolors-core; public Rust/WASM/TypeScript/FFI surface не меняется.Проверено на предфинальном идентичном code tree
cargo fmt --all --check— PASS;cargo clippy --workspace --all-targets --locked -- -D warnings— PASS;cargo test --workspace --locked— PASS;Финальный one-commit head повторно проходит canonical Linux и Swift gates; merge только после их полного GREEN.