feat(source-set): add dependency-aware build order - #50
Conversation
- add optional dependsOn model and schema support - reject invalid dependency graphs before platform launch - validate deep chains with iterative traversal and memoized roots
- order full builds by stable dependency graph - expand scoped builds to transitive source-set closure - preserve legacy and backend build behavior
- verify scoped dependency order, change detection, and failure blocking - preserve yaxunit build-first behavior and result compatibility - document dependsOn workflow and architecture decision
|
Warning Review limit reached
Next review available in: 32 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughДобавлена декларативная связь ChangesЗависимости source-set
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ConfigValidator
participant SourceSetInventory
participant BuildProject
participant PlatformDSL
participant TestRunner
CLI->>ConfigValidator: загрузка v8project.yaml
ConfigValidator-->>CLI: валидированный dependency graph
CLI->>BuildProject: build или test с selection
BuildProject->>SourceSetInventory: построение closure и порядка
SourceSetInventory-->>BuildProject: main -> yaxunit -> TESTS
BuildProject->>PlatformDSL: последовательная сборка
PlatformDSL-->>BuildProject: результат build steps
BuildProject->>TestRunner: запуск после успешного графа
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (4)
tests/cli_build.rs (3)
31-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueДублирование форматной строки скрипта.
write_recording_build_scriptповторяет телоwrite_build_script(строки 15–29) целиком, отличаясь одной строкой записи в лог. При изменении поведения фейкового1cv8придётся править два места.♻️ Вариант объединения helper'ов
-fn write_recording_build_script(path: &Path, calls_log: &Path, fail_pattern: Option<&str>) { - let pattern_branch = fail_pattern - .map(|pattern| { - format!( - "if printf '%s' \"$args\" | grep -F -q -- '{}'; then exit 17; fi", - pattern - ) - }) - .unwrap_or_default(); - let body = format!( - "args=\"$*\"\nout=\"\"\nprev=\"\"\nfor arg in \"$@\"; do\n if [ \"$prev\" = \"/Out\" ]; then out=\"$arg\"; fi\n prev=\"$arg\"\ndone\nprintf '%s\\n' \"$args\" >> '{}'\nif [ -n \"$out\" ]; then printf 'designer log for %s\\n' \"$args\" > \"$out\"; fi\n{}\nexit 0", - calls_log.display(), - pattern_branch - ); - write_script(path, &body); -} +fn write_recording_build_script(path: &Path, calls_log: &Path, fail_pattern: Option<&str>) { + write_build_script_with_log(path, Some(calls_log), fail_pattern); +}
write_build_scriptпри этом становитсяwrite_build_script_with_log(path, None, fail_pattern), а строка записи в лог добавляется в тело только приSome(calls_log).🤖 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 `@tests/cli_build.rs` around lines 31 - 46, Устраните дублирование между write_recording_build_script и write_build_script: выделите общее формирование тела в write_build_script_with_log, принимающий опциональный calls_log и fail_pattern. Добавляйте строку записи аргументов только при Some(calls_log), а существующий write_build_script переведите на этот helper с None, сохранив текущее поведение обоих сценариев.
702-715: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueОбщий корень: нет общего helper'а для разбора лога вызовов
/UpdateDBCfg. Правило «строка с-Extension <name>относится к расширению, иначе к конфигурации» скопировано в три места; при изменении формата аргументов придётся править все.
tests/cli_build.rs#L702-L715: заменить inline-блок вызовом общего helper'а (напримерupdate_db_cfg_order) изtests/support/mod.rs.tests/cli_build.rs#L771-L784: заменить второй inline-блок тем же helper'ом.tests/cli_test.rs#L479-L494: использовать тот же helper, вынеся ветвьRunUnitTests= → enterpriseв параметр или отдельную обёртку.🤖 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 `@tests/cli_build.rs` around lines 702 - 715, Extract the duplicated /UpdateDBCfg call-log parsing into a shared helper in tests/support/mod.rs, such as update_db_cfg_order, preserving the rule that -Extension <name> maps to that extension and other calls map to main. Replace both inline parsing blocks in tests/cli_build.rs at lines 702-715 and 771-784 with the helper, and update tests/cli_test.rs at lines 479-494 to use it while supporting the RunUnitTests= → enterprise distinction through a parameter or dedicated wrapper.
800-804: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueМагический код выхода и хрупкий шаблон фейла.
Some(4)без пояснения затрудняет чтение — стоит сослаться на константу/enum кода ошибки, как это сделано вsrc/domain/test.rs. Кроме того,fail_patternтребует, чтобы/UpdateDBCfgи-Extension yaxunitшли строго подряд в$*; любое добавление аргумента между ними тихо изменит сценарий теста (сборка перестанет падать). Рассмотрите два отдельныхgrepпо подстрокам вместо одной склеенной.Также
setup_dependency_projectуже записывает скрипт, а строки 800–804 сразу его перезаписывают — можно параметризовать helperfail_pattern.Also applies to: 819-820
🤖 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 `@tests/cli_build.rs` around lines 800 - 804, Update the build-script test setup around setup_dependency_project and write_recording_build_script to accept fail_pattern as a helper parameter instead of writing the script again at the call sites. Replace the magic exit status Some(4) with the existing named error-code constant or enum used by the test domain. Make fail_pattern validate /UpdateDBCfg and -Extension yaxunit independently so inserted arguments do not alter the intended failure scenario.tests/cli_test.rs (1)
171-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueТаймаут зашит в helper и дублирует аргумент
setup_project.
execution_timeout_seconds: 5жёстко прописан, хотя тот же 5 передаётся вsetup_project(строка 432). При правке одного значения второе тихо разойдётся. Стоит принять таймаут параметром.♻️ Предлагаемая правка
-fn write_dependency_test_config(path: &Path, work_path: &Path, install_dir: &Path) { +fn write_dependency_test_config( + path: &Path, + work_path: &Path, + install_dir: &Path, + timeout_seconds: u64, +) { let config = format!( - "workPath: '{}'\n...\n execution_timeout_seconds: 5\n...", + "workPath: '{}'\n...\n execution_timeout_seconds: {}\n...", work_path.display(), + timeout_seconds, install_dir.display(), );🤖 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 `@tests/cli_test.rs` around lines 171 - 178, Update write_dependency_test_config to accept an execution-timeout parameter and interpolate it into the generated configuration instead of hardcoding 5. Pass the existing timeout value from setup_project at its call site, keeping the configuration and setup argument synchronized.
🤖 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 `@docs/DEEP_DIVE.md`:
- Around line 66-68: Уточните описание поведения после сбоя зависимости в
разделе документации: замените утверждение, что выполнение останавливается для
всех оставшихся selected nodes, на правило, согласно которому пропускаются
только узлы, зависящие от failed node, а независимые source-set продолжают
выполняться в стабильном порядке.
---
Nitpick comments:
In `@tests/cli_build.rs`:
- Around line 31-46: Устраните дублирование между write_recording_build_script и
write_build_script: выделите общее формирование тела в
write_build_script_with_log, принимающий опциональный calls_log и fail_pattern.
Добавляйте строку записи аргументов только при Some(calls_log), а существующий
write_build_script переведите на этот helper с None, сохранив текущее поведение
обоих сценариев.
- Around line 702-715: Extract the duplicated /UpdateDBCfg call-log parsing into
a shared helper in tests/support/mod.rs, such as update_db_cfg_order, preserving
the rule that -Extension <name> maps to that extension and other calls map to
main. Replace both inline parsing blocks in tests/cli_build.rs at lines 702-715
and 771-784 with the helper, and update tests/cli_test.rs at lines 479-494 to
use it while supporting the RunUnitTests= → enterprise distinction through a
parameter or dedicated wrapper.
- Around line 800-804: Update the build-script test setup around
setup_dependency_project and write_recording_build_script to accept fail_pattern
as a helper parameter instead of writing the script again at the call sites.
Replace the magic exit status Some(4) with the existing named error-code
constant or enum used by the test domain. Make fail_pattern validate
/UpdateDBCfg and -Extension yaxunit independently so inserted arguments do not
alter the intended failure scenario.
In `@tests/cli_test.rs`:
- Around line 171-178: Update write_dependency_test_config to accept an
execution-timeout parameter and interpolate it into the generated configuration
instead of hardcoding 5. Pass the existing timeout value from setup_project at
its call site, keeping the configuration and setup argument synchronized.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1432398b-9f4c-4d2e-aa8d-66587d70ee9e
📒 Files selected for processing (39)
ARCHITECTURE.mdSKILL/SKILL.mdSKILL/references/command-selection.mdSKILL/references/config-and-backends.mdSKILL/references/project-workflows.mdSKILL/references/testing.mddocs/CAPABILITIES.mddocs/CONFIGURATION.mddocs/DEEP_DIVE.mddocs/schemas/v8project.schema.jsonexamples/v8project.yamlspec/architecture/invariants.mdspec/decisions/0023-zavisimosti-source-set-i-stabilnyy-poryadok-build.mdspec/decisions/README.mdsrc/change_detection/source_sets.rssrc/cli/execute.rssrc/config/loader.rssrc/config/model.rssrc/config/schema.rssrc/config/validate.rssrc/mcp/port.rssrc/mcp/server.rssrc/mcp/service.rssrc/platform/edt.rssrc/use_cases/artifacts.rssrc/use_cases/build_project.rssrc/use_cases/check_syntax.rssrc/use_cases/configure_extensions.rssrc/use_cases/dump_config.rssrc/use_cases/extension_identity.rssrc/use_cases/external_artifacts.rssrc/use_cases/init_project.rssrc/use_cases/launch_app.rssrc/use_cases/run_tests.rssrc/use_cases/source_inventory.rssrc/use_cases/transport.rssrc/use_cases/workspace_lock.rstests/cli_build.rstests/cli_test.rs
| уже успешные ранние шаги. Failure dependency останавливает platform execution для всех оставшихся | ||
| selected nodes; они остаются в structured result как `skipped` с причиной | ||
| `aborted after previous failure`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Не останавливайте независимые source-set после сбоя зависимости.
Формулировка «для всех оставшихся selected nodes» противоречит контракту PR: после ошибки должны блокироваться зависимые узлы, но независимые source-set должны по-прежнему обрабатываться в стабильном порядке. Уточните документацию, например: «останавливается выполнение узлов, зависящих от failed node; независимые узлы продолжают выполняться».
Предлагаемая правка
-Failure dependency останавливает platform execution для всех оставшихся
-selected nodes; они остаются в structured result как `skipped` с причиной
+Failure dependency останавливает platform execution для узлов, зависящих от
+неуспешного узла; такие узлы остаются в structured result как `skipped` с причиной
`aborted after previous failure`.📝 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.
| уже успешные ранние шаги. Failure dependency останавливает platform execution для всех оставшихся | |
| selected nodes; они остаются в structured result как `skipped` с причиной | |
| `aborted after previous failure`. | |
| уже успешные ранние шаги. Failure dependency останавливает platform execution для узлов, зависящих от | |
| неуспешного узла; такие узлы остаются в structured result как `skipped` с причиной | |
| `aborted after previous failure`. |
🤖 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 `@docs/DEEP_DIVE.md` around lines 66 - 68, Уточните описание поведения после
сбоя зависимости в разделе документации: замените утверждение, что выполнение
останавливается для всех оставшихся selected nodes, на правило, согласно
которому пропускаются только узлы, зависящие от failed node, а независимые
source-set продолжают выполняться в стабильном порядке.
- Document that failed builds skip all remaining selected source sets\n- Preserve the actual sequential build contract for independent nodes
1 participant
Closes #32.
Verification:
Known baseline: pre-existing unused variable warning in tool_extension.rs and macOS path/timing failures in broad suite.
Summary by CodeRabbit
Новые возможности
dependsOn.Документация