Conversation
be-framework/psalm-plugin 1.x-dev requires vimeo/psalm ^7.0@dev, but stability flags in a dependency's constraints are ignored for transitive resolution; only the root package can allow non-stable versions. Since Psalm 7 has no stable release yet, every fresh 'composer install' (and therefore CI) has been failing with an unresolvable dependency since psalm-plugin adopted the ^7.0@dev constraint upstream on 2026-06-10. Declare vimeo/psalm ^7.0@beta in each demo's require-dev so the root package permits the beta line (currently resolves to 7.0.0-beta19). Verified: all eight demos install from scratch and pass their test suites (178 tests) against the freshly resolved dependency set.
All demos depend on dev branches (be-framework/be 0.x-dev, be-framework/psalm-plugin 1.x-dev), so upstream changes can break a fresh install without any push to this repository. The psalm-plugin constraint change on 2026-06-10 went unnoticed for over a month because CI only ran on push and pull_request. A weekly scheduled run surfaces upstream breakage promptly, and workflow_dispatch allows checking on demand.
The composer dev/profile scripts write timestamped var/log/semantic-dev-*.json files on every run. Only the curated <demo-name>.json logs are meant to be committed; ignore the generated ones so they stop cluttering git status.
Section 6 told agents to run 'composer test' at the repository root and './demos/vendor/bin/phpunit', but there is no root composer.json and no shared demos/vendor directory - each demo is a standalone Composer project. An agent following the contract literally would conclude the test suite is broken. Document the real per-demo workflow, matching README.md and the CI matrix.
Every demo requires php ^8.3 and CI tests 8.3-8.5, but both READMEs still advertised PHP 8.2+.
Four demos (insurance-claim, loan-application, medical-triage, order-processing) had no description, and none of the eight declared a license, so 'composer validate --strict' failed. Add the missing descriptions in the established style and declare MIT (matching the repository license) everywhere. All eight demos now pass strict validation.
…lishing SemanticValidator maps constructor parameter names to PascalCase class names, so ArticleInput's $title and $tags resolve to Semantic\Title and Semantic\Tags. The Semantic tests, however, targeted ArticleTitle and Tag - duplicate orphan classes that nothing in the transformation chain ever invokes. The classes doing the real validation had no direct coverage. Point the tests at Title and Tags, add the missing empty-tags case, and delete the two orphans.
All three Potential classes implement an already-realized guard in be() - the property README advertises as 'Potential idempotency tests' - but no test ever called be() twice, so the guarantee was implemented yet unverified. Cover realization and repeated be() for InventoryReservation, PaymentCapture, and ShippingDispatch using counting closures.
hello-world was the only demo with no Semantic test at all: the Name validator and its InvalidNameException were never exercised, and no test showed what happens when validation fails during metamorphosis. Add a whitespace-only-name rejection test through the real Becoming pipeline plus direct Name validator cases, mirroring medical-triage's style.
CLAUDE.md invariant 4 requires #[Input] parameters before #[Inject] parameters, never interleaved. LoanApproved (Inject, Inject, Input, Input, Inject) and MetadataResolved (Inject, Input, Input, Inject) both violated the contract they are meant to exemplify. Reorder the parameters and switch the hand-built LoanApproved test constructions to named arguments so the call sites no longer depend on parameter order.
…cription password_hash() has returned string unconditionally since PHP 8.0 (failure raises ValueError), so the false-check throwing a generic RuntimeException was unreachable - and generic exceptions are against house style anyway. blog-publishing's composer description advertised 'Moment without Potential and mini-diamond merge', which describes classes that are not wired into the live chain; the demo's actual documented pattern is the Multi-Reason Being.
The blanket claim that every demo ships Reason layer tests was untrue - only the four advanced demos have them. Describe the real distribution: happy-path plus Semantic tests everywhere, Reason and Potential idempotency tests in the advanced demos.
The invariant listed 'clock, randomness' as external I/O boundaries requiring an interface, yet the demos consistently inject in-process generators concretely (UserIdGenerator, WelcomeTokenGenerator, ReceiptGenerator, PublishTimestamper - all use date()/random_bytes(), none has an interface) while reserving interfaces for Reasons that model external systems. An agent applying the written rule would 'fix' half the catalog. Document the actually-observed boundary: external systems need interfaces, in-process policies/calculators/generators do not. Continues the direction of e9e9c35.
createFromFormat('m/y') fills the unspecified day and time from 'now',
which broke the check in two directions:
- On the 29th-31st, an expiry in a shorter month overflowed into the
next month (06/26 read on July 31 becomes 2026-07-01), so an expired
card passed validation.
- A card expiring in the current month carried the current clock time,
so on the last day of the month it compared as already expired.
Use '!Y-m' (day and time reset) and compare first-of-month to
first-of-month, the same semantics CardValidator already implements
correctly. Add the missing CardExpiryTest covering format errors,
expired cards, and the current-month boundary.
PHILOSOPHY.md and the After_BeFramework comparison showed OrderConfirmed taking its Moments via a #[Moment] attribute that exists nowhere in the framework - copying the example verbatim fatals with an unknown attribute class. The real OrderConfirmed uses #[Inject]; the docs now match it.
The Complex Convergence description said each Input resolves to one of the two Finals 'by $being type matching' - wording copied from the medical-triage entry. insurance-claim has no $being discriminator anywhere; only medical-triage implements that mechanism. Describe what the #[Be] attribute actually declares instead.
…lback SemanticValidator resolves a validator class from the constructor parameter name, so ClaimInput's $estimatedAmount looks for EstimatedAmount and LoanInput's $requestedAmount looks for RequestedAmount. The classes were named ClaimAmount and LoanAmount after the business concept instead, so neither could ever be resolved. Rename them to match their parameters (their validate() signatures already did). Also make IncomePolicy's assessment fallback null-based: with '$this->lastDti ?: 0.25' a legitimately computed DTI of 0.0 (huge income, minimal loan) was silently replaced by the 0.25 fallback. Track the not-yet-computed state as null and use ?? instead, with a regression test.
The template showed #[Input] for a Moment's scalar fields while citing InventoryReserved as canonical - but that class (like every real Moment in the catalog) binds its scalars with per-demo qualifier attributes (#[ProductId], #[Quantity], ...) from AppModule, not #[Input]. Add the distinction so a copied template doesn't teach the one pattern its cited example avoids.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 51 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. 📝 WalkthroughWalkthroughThe changes update repository automation and documentation, add Psalm tooling across demos, rename several semantic validators, adjust dependency injection ordering, preserve zero-valued loan policy results, refine card-expiry and password-hashing behavior, and expand demo test coverage. ChangesRepository guidance and tooling
Semantic and runtime behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
demos/blog-publishing/src/Moment/MetadataResolved.php (1)
23-37: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy liftImplement
MomentInterfaceand define abe()method.This class is located in the
src/Moment/directory but does not implementMomentInterface, create a Potential, or define abe()method. As per coding guidelines, all Moments must adhere to these invariants.If this class is intended to be a
Being(since it appears to only compute properties without external side effects), it should be moved to thesrc/Being/directory and its namespace updated accordingly.🤖 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 `@demos/blog-publishing/src/Moment/MetadataResolved.php` around lines 23 - 37, Update MetadataResolved to implement MomentInterface and add the required be() method, creating and returning the appropriate Potential while preserving its existing injected property resolution. If it is intended to remain a side-effect-free Being, instead move it to the Being namespace and directory and apply the corresponding Being contract.Source: Coding guidelines
🧹 Nitpick comments (1)
demos/hello-world/tests/HelloTest.php (1)
55-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
expectNotToPerformAssertions()here.
$this->expectNotToPerformAssertions();is a clearer PHPUnit idiom thanaddToAssertionCount(1)for this no-assertion path.♻️ Proposed refactor
public function testValidName(): void { + $this->expectNotToPerformAssertions(); $semantic = new Name(); $semantic->validate('World'); - $this->addToAssertionCount(1); }🤖 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 `@demos/hello-world/tests/HelloTest.php` around lines 55 - 60, Update HelloTest::testValidName to call PHPUnit’s expectNotToPerformAssertions() before validating the name, and remove the addToAssertionCount(1) workaround while preserving the existing Name validation.Sources: Coding guidelines, Learnings
🤖 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 `@demos/blog-publishing/src/Moment/MetadataResolved.php`:
- Around line 23-37: Update MetadataResolved to implement MomentInterface and
add the required be() method, creating and returning the appropriate Potential
while preserving its existing injected property resolution. If it is intended to
remain a side-effect-free Being, instead move it to the Being namespace and
directory and apply the corresponding Being contract.
---
Nitpick comments:
In `@demos/hello-world/tests/HelloTest.php`:
- Around line 55-60: Update HelloTest::testValidName to call PHPUnit’s
expectNotToPerformAssertions() before validating the name, and remove the
addToAssertionCount(1) workaround while preserving the existing Name validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 488087e6-f904-4289-a816-556ffc1954ac
📒 Files selected for processing (35)
.github/workflows/tests.yml.gitignoreCLAUDE.mdREADME.ja.mdREADME.mddemos/blog-publishing/composer.jsondemos/blog-publishing/src/Moment/MetadataResolved.phpdemos/blog-publishing/src/Semantic/ArticleTitle.phpdemos/blog-publishing/src/Semantic/Tag.phpdemos/blog-publishing/tests/BlogPublishingTest.phpdemos/contact-form/composer.jsondemos/hello-world/composer.jsondemos/hello-world/tests/HelloTest.phpdemos/insurance-claim/README.mddemos/insurance-claim/composer.jsondemos/insurance-claim/src/Semantic/EstimatedAmount.phpdemos/insurance-claim/tests/InsuranceClaimTest.phpdemos/loan-application/README.mddemos/loan-application/composer.jsondemos/loan-application/src/Final/LoanApproved.phpdemos/loan-application/src/Reason/IncomePolicy.phpdemos/loan-application/src/Semantic/RequestedAmount.phpdemos/loan-application/tests/LoanApplicationTest.phpdemos/medical-triage/composer.jsondemos/order-processing/composer.jsondemos/order-processing/docs/PHILOSOPHY.mddemos/order-processing/docs/comparison/After_BeFramework.phpdemos/order-processing/src/Semantic/CardExpiry.phpdemos/order-processing/tests/Moment/Potential/InventoryReservationTest.phpdemos/order-processing/tests/Moment/Potential/PaymentCaptureTest.phpdemos/order-processing/tests/Moment/Potential/ShippingDispatchTest.phpdemos/order-processing/tests/Semantic/CardExpiryTest.phpdemos/user-registration/composer.jsondemos/user-registration/src/Reason/PasswordHasher.phpdocs/templates/MomentTemplate.php
💤 Files with no reviewable changes (2)
- demos/blog-publishing/src/Semantic/Tag.php
- demos/blog-publishing/src/Semantic/ArticleTitle.php
Invariant 5 stated all Moments implement MomentInterface, but five Moments across the catalog (FraudCleared, AdjustmentReviewed, EscalationQueued, IncomeApproved, MetadataResolved) are deliberate pure-data Moments - parts of a whole with no Potential and no be() - each self-documented in its docblock. CodeRabbit flagged MetadataResolved against the written rule, which confirms the rule as stated misleads reviewers and agents. Document the variant instead of 'fixing' five intentional classes.
|
@coderabbitai Addressed in 9f47a6e: MetadataResolved is one of five deliberate pure-data Moments in the catalog (FraudCleared, AdjustmentReviewed, EscalationQueued, IncomeApproved are the others) — no Potential to realize, self-documented in the docblock. The gap was in CLAUDE.md invariant 5, which didn't mention the variant; it now does. Skipping the expectNotToPerformAssertions() nitpick: addToAssertionCount(1) is the established idiom across all eight demos' test suites, and a single divergent test would hurt consistency. |
|
Tip For best results, initiate chat on the files or code changes.
🐇💭 ✏️ Learnings added
|
This branch has not been deployed
1 participant
Why
A fresh
composer installin every demo has been failing since 2026-06-10:psalm-plugin's Psalm 7 support (be-framework/psalm-plugin#3) switched its requirement to
vimeo/psalm ^7.0@dev. Composer ignores stability flags in transitive constraints — only the root package may allow non-stable versions — and Psalm 7 has no stable release, so resolution fails on every fresh clone. CI never noticed because the last run predates the upstream change and CI only fires on push/PR.While fixing that, a full audit of the catalog (docs-vs-code, CLAUDE.md invariants, test coverage, code quality) surfaced further issues, fixed in the follow-up commits.
What
Repo health
vimeo/psalm: ^7.0@betain require-dev so the root permits the beta line (resolves to 7.0.0-beta19). Verified: all eight demos install from scratch and pass.schedule+workflow_dispatch— the demos depend on dev branches, so upstream changes can break a fresh install without any push here.var/log/semantic-dev-*.jsondev logs.composer validate --strictnow passes in all demos.Documentation accuracy (this repo is an AI-assistant contract)
composer testat the root and./demos/vendor/bin/phpunit— neither path exists. Now documents the real per-demo workflow.$beingtype-matching mechanism that only medical-triage implements.#[Moment]attribute — copying it verbatim fatals. Replaced with the real#[Inject].Bugs
createFromFormat('m/y')fills day/time from "now", so on the 29th–31st an expiry in a shorter month overflowed into the next month (expired card accepted), and a card expiring in the current month was rejected on the month's last day. Now uses!Y-m+ first-of-month comparison, the semantics CardValidator already had. Added the missing CardExpiryTest.$this->lastDti ?: 0.25silently replaced a legitimately computed DTI of 0.0 (huge income, small loan) with the fallback. Now null-tracked with??, plus a regression test.password_hash() === falsebranch (impossible since PHP 8.0) and its generic RuntimeException.Invariant violations
LoanApprovedandMetadataResolvedinterleaved#[Input]/#[Inject]. Reordered; hand-built test constructions switched to named arguments.$title→Titleand$tags→Tags, but the tests targeted duplicate orphansArticleTitle/Tag— the live validators had no direct coverage. Tests now target the real classes; orphans deleted.ClaimAmountandLoanAmountvalidated$estimatedAmount/$requestedAmountbut can never be resolved from those parameter names. Renamed toEstimatedAmount/RequestedAmount.Test gaps closed
be()twice — now covered.Verification
All 8 demos, fresh install (
rm composer.lock && composer install) + PHPUnit: green, 195 tests (baseline was 178).Known limitations (out of scope, flagged separately)
composer psalmstill fatals: psalm-plugin'sInputTaintHandler::addTaints(): arraypredates Psalm 7's int-bitflag taint API (beta18+). That fix belongs in be-framework/psalm-plugin.#[Be]chains that bypass their documented Being layers (orphaned classes, hardcoded Moment inputs, an unreachableClaimEscalated, and aTypeErrorif loan-application is run through the realBecoming). Rewiring them is a design decision left for a dedicated follow-up.🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Bug Fixes
Tests
Chores