STO-399: build TypeScript graph before unit lane - #96
Conversation
|
Warning Review limit reached
Next review available in: 11 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ 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 |
|
Verdict: approve stometa, no findings at What I checked:
Verification re-run locally under Node 22.12.0/npm 10.9.0 with finite timeouts: Accepted choice: Approve the exact head because Rejected alternatives:
Constraint: The issue requires the smallest deterministic repair to the documented clean-install/unit path without changing dependencies, lifecycle scripts, or product behavior. |
|
Verdict: pass stometa, no findings at immutable head Accepted choice: Pass this exact head because the documented clean install/unit path now provisions the existing SDK → ActionPlane → BotDelivery graph and every required independent gate passed. Rejected alternatives:
Constraint: STO-399 requires the smallest deterministic repair to the documented Security findings: none. Performance findings: none. Measured clean-state Evidence checks:
Skipped checks: none. |
|
Verdict: approve stometa, no blocking findings at immutable PR head What I checked:
Verification re-run locally with finite timeouts: One preliminary install wrapper used zsh's reserved read-only variable SKIPPED locally — Blocking findings: none. Accepted choice: Approve PR #96 at the exact reviewed head because the smallest Make prerequisite repair structurally and behaviorally fixes the clean-checkout unit lane without changing product behavior. Rejected alternatives:
Constraint: STO-399 requires the documented |
Originating Multica issue: STO-399
Original author: @ryan
Summary
The documented Python unit lane now provisions the local TypeScript build artifacts it executes:
make testbuilds the existing SDK → ActionPlane → BotDelivery dependency graph before pytest (Makefile:20). A clean checkout no longer requires undocumented manual builds.Why
make installintentionally installs TypeScript dependencies with--ignore-scripts(Makefile:4-7), but the embedded BotDelivery contract test builds BotDelivery directly (tests/unit/test_bot_delivery_model_egress_contract.py:46-60). BotDelivery resolves local peer packages through theirdistexports, so the documented clean-checkout unit command failed when ActionPlane had not already been built. Closes STO-399.Approach
Accepted choice: Declare the existing
bot-buildtarget as the prerequisite oftest;bot-buildalready expresses SDK + ActionPlane → BotDelivery (Makefile:20,Makefile:82-83).Rejected alternatives:
npm ci: rejected because the repository intentionally installs with--ignore-scripts(Makefile:5-7).bot-build.Constraint: The fix must preserve package versions, runtime behavior, public APIs, and the existing isolated package gates while making the documented
make install+make testpath deterministic.The regression test compares exact Make target lines, preventing substring false positives and locking both the unit-lane prerequisite and the existing local dependency graph (
tests/unit/test_bot_delivery_application_contract.py:84-89). No dependency or product code changed.How I Tested
End-to-end test cases
tests/unit/test_bot_delivery_application_contract.py:84test: bot-buildandbot-build: sdk-build action-buildtarget contracts exist.tests/unit/test_bot_delivery_model_egress_contract.py:46Verbatim test output
Red on baseline Makefile with the new regression test:
Green clean dependency reproduction and full unit lane:
Existing tests
gtimeout 300 make buildSuccessfully built dist/context_engine-0.1.0.tar.gzand wheelgtimeout 600 make test1300 passed in 23.36sgtimeout 600 make catalog132 passed in 15.50s;security catalog valid: 15 invariants, 12 fixturesgtimeout 300 make openapi-checkgtimeout 300 make openapi-breaking-check9 passed in 0.60sgtimeout 600 make sdk-testpacked SDK consumer passedgtimeout 300 make sdk-packcontext-engine-resolve-sdk-0.0.0-v0.tgzproduced in ignored build outputgtimeout 600 make action-testpacked ActionPlane consumer passedgtimeout 600 make bot-testpacked BotDelivery consumer passedgtimeout 1200 make integration445 passed, 4 deselected, 2 warnings in 302.06s (0:05:02)gtimeout 600 make smoke6 passed in 3.48sgtimeout 1200 make security-gate157 passed in 106.58s;M0 SECURITY PASSIntegration emitted two existing
PytestUnraisableExceptionWarningwarnings from SQLAlchemy event-registry weakref cleanup; there were no test failures.Cleanup proof:
New tests added
tests/unit/test_bot_delivery_application_contract.py:84— proves the documented unit lane owns the complete local TypeScript dependency build order.Lint / typecheck
Definition of Done
origin/main29b4a6b3828b484aea9b5cc8f31dcef290e47caf.make installleaves zerodisttrees;make testprovisions SDK → ActionPlane → BotDelivery deterministically under Node 22.12.0/npm 10.9.0.Makefileand one unit contract test;git diff --checkpasses; no untracked files, generated artifacts, credentials, environment files, or unrelated changes are included.Rollback Plan
Revert commit
cab629171e15378db091388a5b74e40d2dfc3c0d. Maximum blast radius is the local/CI verification command graph only; no production runtime or data is touched. Time to rollback is under five minutes plus CI validation.Residual risk:
make testnow performs three deterministic TypeScript builds, increasing unit-lane duration by several seconds. This is bounded to developer/CI verification and uses existing build targets; no runtime residual risk remains.Out of Scope
make checkgraph.