O1b-2: переиспользовать runtime-арены Session - #482
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 27 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 (25)
WalkthroughИзменения переводят observation и program evaluation на переиспользуемые арены, переносят deferred retirement в ChangesАрены наблюдений
Арены оценки программы
Жизненный цикл сессии
Allocator sink
Контракты и validation
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 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: 9
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/program_session.rs (1)
2550-2562: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winСделайте проверку preflight-отказа до раннего
return. На warm-pathcapacity() >= capacityhelper выходит без вызоваinjected_program_preflight_failure(), поэтому после первого commit счётчикreservation_indexперестаёт продвигаться, и сценарии сfail_program_preflight_reservation_for_test(...)больше не попадают в ожидаемый слот.🤖 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_session.rs` around lines 2550 - 2562, Переместите проверку injected_program_preflight_failure() в try_reserve_program_evaluation_buffer перед ранним return по условию buffer.capacity() >= capacity. Сохраните возврат Ok(()) для warm-path после выполнения проверки, чтобы reservation_index продолжал продвигаться и fail_program_preflight_reservation_for_test(...) попадал в ожидаемый слот.
🤖 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/generic_boundary_tests.rs`:
- Around line 120-126: Update the gate in the generic boundary test around the
syntax, pool, and constructor scans to count the `ObservationBackingV1`
declaration separately, then assert that no `ObservationBackingV1{` literals
exist outside `ObservationArenaPoolV1::new`. Remove the fragile global
`syntax.matches(...).count() == 2` dependency while preserving the requirement
for exactly one production constructor using `Rc::new(ObservationBackingV1{`.
In `@crates/labcolors-core/src/observation.rs`:
- Around line 186-193: Уточните doc-комментарий над ObservedScenarioSet: явно
обозначьте, что пустое состояние создаётся методом empty() только для
прогретого, ещё не арендованного слота пула, тогда как канонический набор
связанных сценариев непуст.
- Around line 1104-1110: Update try_reserve_total so the reserve amount is
calculated without relying on an implicit capacity >= len invariant, or
explicitly document and validate that invariant before subtracting. Preserve the
existing ResourceExhausted mapping and successful no-op behavior when storage
already has sufficient capacity.
In `@crates/labcolors-core/src/program_session.rs`:
- Around line 2118-2129: Replace the unconditional unreachable! check in
into_arena with debug_assert_eq! for the observation and arena slots, then
continue returning the lease-backed arena after dropping observation. Preserve
restore’s existing double-return validation and avoid introducing a panic in the
retirement commit/rollback path.
- Around line 2641-2654: Replace the unconditional unreachable! check in
ProgramEvaluationArenaGuardV1::drop with debug_assert!, allowing cleanup during
unwinding without triggering a second panic; keep the lease restoration behavior
unchanged and preserve unconditional invariant enforcement in restore.
In `@crates/labcolors-core/src/program/attachment/support.rs`:
- Around line 548-552: В `prepare()` замените `unwrap_or_else(||
unreachable!(...))` при получении `self.shared.stamp` на возврат соответствующей
типизированной ошибки через `Self::Error`. Сохраните успешный путь с
существующим `base_stamp`, а отсутствие `stamp` должно завершать `prepare()`
контролируемым `Err` без паники.
In `@crates/labcolors-core/src/session.rs`:
- Around line 337-350: Усильте проверку deferred_retirement в
SessionBuilder::commit, чтобы она соответствовала безусловной проверке в
commit_deferred и не исчезала в release-сборке: замените
debug_assert!(deferred_retirement.is_none()) на эквивалентную обязательную
проверку с unreachable-поведением. Сохраните остальную последовательность
публикации перехода и retirement без изменений.
- Around line 237-255: Переименуйте активно используемые поля
DeferredSessionRetirement _retired_verified, _retired_violation,
_retired_raw_head и _displaced_placeholder, убрав ведущие подчёркивания, и
обновите все их обращения в retire_into и других местах реализации. Не
переименовывайте _owner, поскольку он по-прежнему нужен исключительно для
порядка drop.
In `@scripts/test_program_public_surface.py`:
- Around line 56-88: Добавьте в тестовый класс сценарий с синтетической
страницей внутри запрещённого модуля, например
`session/struct.SessionItem.html`, используя `write_all` и `write_item` по
аналогии с `test_arbitrary_session_alias_is_rejected_by_origin`. Вызовите
`program_public_surface` и проверьте, что такой путь фиксируется ровно как один
leak, покрывая ветку `_inside(page, module)`.
---
Outside diff comments:
In `@crates/labcolors-core/src/program_session.rs`:
- Around line 2550-2562: Переместите проверку
injected_program_preflight_failure() в try_reserve_program_evaluation_buffer
перед ранним return по условию buffer.capacity() >= capacity. Сохраните возврат
Ok(()) для warm-path после выполнения проверки, чтобы reservation_index
продолжал продвигаться и fail_program_preflight_reservation_for_test(...)
попадал в ожидаемый слот.
🪄 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: bf426cb7-e312-48de-80a8-67d359245ad4
📒 Files selected for processing (20)
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/joint_tests.rscrates/labcolors-core/src/observation.rscrates/labcolors-core/src/observation_tests.rscrates/labcolors-core/src/point_support_tests.rscrates/labcolors-core/src/program.rscrates/labcolors-core/src/program/attachment.rscrates/labcolors-core/src/program/attachment/support.rscrates/labcolors-core/src/program/attachment/tests.rscrates/labcolors-core/src/program_joint_integration_tests.rscrates/labcolors-core/src/program_mixed_evaluator_tests.rscrates/labcolors-core/src/program_point_causality_tests.rscrates/labcolors-core/src/program_session.rscrates/labcolors-core/src/program_session_tests.rscrates/labcolors-core/src/session.rscrates/labcolors-core/src/session_tests.rsscripts/test_program_public_surface.pyscripts/verify_program_public_surface.py
|
Outside-diff finding Fault-injection index теперь отражает логическую последовательность preflight-координат, а не случайную warm/cold topology: каждая ненулевая координата потребляет индекс до early return, нулевая не считается reservation, а injected OOM реализуется только там, где действительно нужен рост буфера. Это закреплено тремя helper-level mutation controls и end-to-end тестом Exact-head evidence:
|
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/program_session.rs`:
- Around line 2736-2745: Добавьте явные комментарии с обоснованием инвариантов
перед обоими вызовами unreachable! в ProgramEvaluationArenaV1::restore: слот,
полученный из observation, всегда валиден и находится в ограниченной арене, а
move-only arena не допускает повторного возврата. Сохраните текущую логику
restore и формулировки, согласованные с пояснением в соседнем Drop.
🪄 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: 69f8fd15-cc8b-4215-bfe2-979ffd997cf1
📒 Files selected for processing (15)
.github/workflows/ci.ymlcrates/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/generic_boundary_tests.rscrates/labcolors-core/src/observation.rscrates/labcolors-core/src/program/attachment/support.rscrates/labcolors-core/src/program/attachment/tests.rscrates/labcolors-core/src/program_joint_integration_tests.rscrates/labcolors-core/src/program_session.rscrates/labcolors-core/src/session.rspackages/colors/test/release-contract.test.mjsscripts/test_point_support_surplus_source_binding.pyscripts/test_program_public_surface.pyscripts/verify_point_support_surplus.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
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)
.github/workflows/ci.yml (1)
541-572: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winСделайте выбор ALSA-пакета зависимым от
VERSION_CODENAME.
В.github/workflows/ci.yml:572сейчас всегда запрашиваетсяlibasound2t64, но на Ubuntu 22.04 (Jammy) доступен толькоlibasound2;libasound2t64появляется лишь начиная с Ubuntu 24.04 (Noble). На Jammy этот шаг уронитapt-get download, поэтому нужен выбор по codename или явный отказ для неподдерживаемых релизов.🤖 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 @.github/workflows/ci.yml around lines 541 - 572, Update the Chrome dependency download block using VERSION_CODENAME so Jammy requests libasound2 and Noble or later requests libasound2t64; preserve the existing libnspr4, libnss3, and libgbm1 downloads, and explicitly reject unsupported codenames before apt-get download.
🤖 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 `@packages/colors/test/release-contract.test.mjs`:
- Around line 524-530: Расширьте контрактные проверки для содержимого APT
sources в переменной active: добавьте assertions для обеих веток case "$ID"
(Debian и Ubuntu), подтверждающие официальные vendor URL, $VERSION_CODENAME и
signed-by=$DISTRO_KEYRING. Добавьте негативные проверки, которые отклоняют
подмену URL, keyring, suite и signed-by, чтобы изменения trusted sources не
проходили тест.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 541-572: Update the Chrome dependency download block using
VERSION_CODENAME so Jammy requests libasound2 and Noble or later requests
libasound2t64; preserve the existing libnspr4, libnss3, and libgbm1 downloads,
and explicitly reject unsupported codenames before apt-get download.
🪄 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: 905e8041-c3f5-4f97-a8b4-02ca11d3b391
📒 Files selected for processing (5)
.github/workflows/ci.ymlcrates/labcolors-core/contracts/clean-set-srgb8-v1/receipt-v1.jsoncrates/labcolors-core/contracts/clean-set-srgb8-v1/receipt-v1.sha256crates/labcolors-core/src/program_session.rspackages/colors/test/release-contract.test.mjs
|
Outside-diff finding |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@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 29 minutes. |
|
@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 49 seconds. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/observation.rs`:
- Around line 322-340: Упростите materialize_into, убрав повторный вызов
Rc::get_mut для уже выбранного слота: после определения slot_index получите
единственную изменяемую ссылку на self.slots[slot_index] и используйте её для
materialize и клонирования. Удалите заведомо недостижимую обработку
InternalInvariant, сохранив текущую ошибку только для случая отсутствия
свободного слота.
- Around line 1106-1130: In the scenario-grouping loop within the visible
case-building flow, replace the scenarios.peek() condition followed by
scenarios.next().unwrap_or_else(...) with Peekable::next_if, consuming the next
ScenarioInput only when its bindings match the current bindings. Preserve
pushing the consumed id to set.provenance and remove the unnecessary
unreachable! handling.
In `@crates/labcolors-core/src/program/attachment/support.rs`:
- Around line 566-573: Уточните семантику
InMemoryPointSinkErrorV1::StampMismatch: задокументируйте, что ошибка
возвращается как при отсутствии ожидаемого patch, так и при несовпадении
patch.sink_output() с self.owned_scope[0]. Если вызывающим сторонам требуется
различать эти случаи, добавьте отдельный вариант ошибки и возвращайте его в
соответствующей ветке проверки.
In `@crates/labcolors-core/src/session.rs`:
- Around line 284-291: Update ProgramEvaluationArenaPoolV1::restore so
double-return and invalid-index checks cannot unconditionally panic when reached
through PendingSessionTransitionGuard::drop during unwinding. Return the
function’s existing typed unreachable/error context for these cases, optionally
retaining debug_assert! for invariant diagnostics, and leave normal arena
restoration unchanged.
🪄 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: f589a075-2c8e-4e6b-bad0-08fae7d17a18
📒 Files selected for processing (25)
.github/workflows/ci.ymlcrates/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/generic_boundary_tests.rscrates/labcolors-core/src/joint_tests.rscrates/labcolors-core/src/observation.rscrates/labcolors-core/src/observation_tests.rscrates/labcolors-core/src/point_support_tests.rscrates/labcolors-core/src/program.rscrates/labcolors-core/src/program/attachment.rscrates/labcolors-core/src/program/attachment/support.rscrates/labcolors-core/src/program/attachment/tests.rscrates/labcolors-core/src/program_joint_integration_tests.rscrates/labcolors-core/src/program_mixed_evaluator_tests.rscrates/labcolors-core/src/program_point_causality_tests.rscrates/labcolors-core/src/program_session.rscrates/labcolors-core/src/program_session_tests.rscrates/labcolors-core/src/session.rscrates/labcolors-core/src/session_tests.rspackages/colors/test/release-contract.test.mjsscripts/test_point_support_surplus_source_binding.pyscripts/test_program_public_surface.pyscripts/verify_point_support_surplus.pyscripts/verify_program_public_surface.py
|
@coderabbitai full review Повторная проверка точного head |
|
✅ 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 27 minutes. |
1 participant
Что изменено
previous + cause + prospective.Vec-буферы; Program использует одну arena с покомпонентным максимумом selected/exhaustive ёмкости.Почему
До среза каждый новый observed update создавал новый
Rc<ObservationBackingV1>и отделял report/output buffers. Whole-update RED после warm-up зафиксировалAllocatorEvents { alloc: 9, realloc: 0, dealloc: 0 }. Простая отсрочка drop в Attachment скрывала момент освобождения, но не переиспользовала storage.Общий закон — один логический slot на одну revision-bound пару observation+evaluation storage; минимум три слота выводится из автомата Failed(cause + previous) + prospective.
Влияние
После разогрева полный Attachment lifecycle — Ready/Failed/Ready, Unknown/Stale, ConfirmExact, три sink rejection и retry — выполняется с нулём alloc/alloc_zeroed/realloc/dealloc. Семантика lifecycle, численные результаты и публичная поверхность не меняются.
Проверки
cargo test --workspace --lockedcargo clippy --workspace --all-targets --locked -- -D warningscargo fmt --all -- --checkcargo check --workspace --all-targets --lockedRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --lockedpublic_items=155Plan(InternalInvariant)Summary by CodeRabbit
Улучшения
Исправления
Тесты
Chores