feat!: add strict recipe execution and application-scoped tag configuration without new global mutable state.#100
Conversation
…ration without new global mutable state.
|
Warning Review limit reached
Next review available in: 46 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR replaces global factory defaults with application-scoped configuration recipes, adds strict recipe execution and validation, applies theme recipes during component creation, centralizes Fiber-aware stack storage, and updates tests, documentation, and migration guidance. ChangesRecipe configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
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 |
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Config/Config.php (1)
58-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPropagated
ConfigExceptionis undocumented at both config-application entry points.ConfigApplierInterface::apply()can throwConfigExceptionin strict mode, but neither call-chain layer's docblock reflects that.
src/Config/Config.php#L58-L81: add@throws ConfigException If a resolved recipe cannot be applied to the created component.tocreate()'s docblock (and consider the same forapply(), since it delegates to the same applier).src/Base/BaseTag.php#L145-L180: extendconfig()'s@throws ConfigExceptionto also cover failures propagated from$config->apply(), not just the local incompatible-component case.🤖 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 `@src/Config/Config.php` around lines 58 - 81, Document propagated ConfigException at both entry points: in src/Config/Config.php lines 58-81, add it to create() and the delegating apply() docblocks as applicable, describing failures applying resolved recipes; in src/Base/BaseTag.php lines 145-180, update config()’s `@throws` ConfigException description to include exceptions propagated from $config->apply(), while retaining the existing incompatible-component case.
🤖 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 `@CHANGELOG.md`:
- Line 11: Release the breaking removal of SimpleFactory defaults APIs as
version 0.7.0: move the entry in CHANGELOG.md at lines 11-11 under the 0.7.0
release section, and rename the migration section in UPGRADE.md at lines 3-20 to
0.7.0.
In `@tests/Element/TagBlockTest.php`:
- Around line 143-180: Update testMaintainsIndependentBeginEndStackForFiber to
capture the existing BaseBlock::$stack and $mainThread values before resetting
them, then restore both values in a finally block surrounding the test setup and
assertions. Preserve the current Fiber behavior and assertions while ensuring
cleanup runs when any step fails.
---
Outside diff comments:
In `@src/Config/Config.php`:
- Around line 58-81: Document propagated ConfigException at both entry points:
in src/Config/Config.php lines 58-81, add it to create() and the delegating
apply() docblocks as applicable, describing failures applying resolved recipes;
in src/Base/BaseTag.php lines 145-180, update config()’s `@throws` ConfigException
description to include exceptions propagated from $config->apply(), while
retaining the existing incompatible-component case.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1bc953be-58cc-4b89-80ef-58b7d2f7aa96
⛔ Files ignored due to path filters (2)
docs/svgs/features-mobile.svgis excluded by!**/*.svgdocs/svgs/features.svgis excluded by!**/*.svg
📒 Files selected for processing (25)
CHANGELOG.mdREADME.mdUPGRADE.mdcomposer.jsonsrc/Base/BaseTag.phpsrc/Config/Config.phpsrc/Config/ConfigApplier.phpsrc/Config/ConfigApplierInterface.phpsrc/Config/Recipe.phpsrc/Element/BaseBlock.phpsrc/Exception/ConfigException.phpsrc/Exception/Message.phpsrc/Factory/SimpleFactory.phpsrc/Theme/ThemeInterface.phptests/Base/BaseTagTest.phptests/Config/ConfigApplierTest.phptests/Config/ConfigTest.phptests/Element/TagBlockTest.phptests/Element/TagInlineTest.phptests/Element/TagInputTest.phptests/Element/TagVoidTest.phptests/Factory/SimpleFactoryTest.phptests/Support/Stub/ConfigApplier.phptests/Support/Stub/ConfigurableComponent.phptests/Support/Stub/ExtendedComponent.php
💤 Files with no reviewable changes (3)
- tests/Element/TagInputTest.php
- tests/Element/TagVoidTest.php
- tests/Element/TagInlineTest.php
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: phpunit / PHP 8.4-windows-2022
- GitHub Check: phpunit / PHP 8.3-windows-2022
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-09T21:17:27.239Z
Learnt from: terabytesoftw
Repo: ui-awesome/html-core PR: 48
File: composer.json:14-14
Timestamp: 2026-01-09T21:17:27.239Z
Learning: In the ui-awesome/html-core repository, during active development phases it is acceptable to use development version constraints in composer.json (for example '^0.6dev') to facilitate rapid iteration. Reviewers should tolerate dev constraints here, but plan to revert to stable versions once the project stabilizes. When reviewing, ensure such constraints are clearly documented in the commit message or PR description and that a follow-up task exists to switch to stable versions before release.
Applied to files:
composer.json
📚 Learning: 2025-12-19T22:28:15.476Z
Learnt from: terabytesoftw
Repo: ui-awesome/html-core PR: 16
File: src/Exception/Message.php:59-61
Timestamp: 2025-12-19T22:28:15.476Z
Learning: In ui-awesome/html-core/src/Exception/Message.php, for the KEY_MUST_BE_NON_EMPTY_STRING error, use a simplified, concise message without diagnostic placeholders. Since the error context (empty string or non-string type) is already sufficient for debugging, avoid embedding runtime values or extra type details in the message. Example: replace messages like 'Key must be a non-empty string: value=... type=...' with 'Key must be a non-empty string.' Ensure consistency across similar validation errors and avoid leaking internal debugging details in production.
Applied to files:
src/Exception/Message.php
🪛 PHPMD (2.15.0)
tests/Factory/SimpleFactoryTest.php
[error] 24-24: Avoid using static access to class '\UIAwesome\Html\Core\Factory\SimpleFactory' in method 'testConfigureIgnoresNumericKeys'. (undefined)
(StaticAccess)
[error] 24-24: Avoid using static access to class '\UIAwesome\Html\Core\Tests\Support\Stub\TagInline' in method 'testConfigureIgnoresNumericKeys'. (undefined)
(StaticAccess)
src/Config/ConfigApplier.php
[error] 40-40: The method apply has a boolean flag argument $strict, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
tests/Config/ConfigApplierTest.php
[warning] 18-337: The class ConfigApplierTest has 13 public methods. Consider refactoring ConfigApplierTest to keep number of public methods under 10. (undefined)
(TooManyPublicMethods)
tests/Element/TagBlockTest.php
[error] 156-156: Avoid using static access to class '\Fiber' in method 'testMaintainsIndependentBeginEndStackForFiber'. (undefined)
(StaticAccess)
[error] 157-157: Avoid using static access to class '\UIAwesome\Html\Core\Tests\Support\Stub\TagBlock' in method 'testMaintainsIndependentBeginEndStackForFiber'. (undefined)
(StaticAccess)
[error] 176-176: Avoid using static access to class '\UIAwesome\Html\Core\Tests\Support\Stub\TagBlock' in method 'testMaintainsIndependentBeginEndStackForFiber'. (undefined)
(StaticAccess)
🔇 Additional comments (20)
src/Element/BaseBlock.php (1)
120-121: LGTM!Also applies to: 162-164, 242-259
src/Factory/SimpleFactory.php (1)
14-14: LGTM!Also applies to: 53-58
tests/Factory/SimpleFactoryTest.php (1)
22-35: LGTM!tests/Element/TagBlockTest.php (1)
7-7: LGTM!Also applies to: 58-74
README.md (1)
269-326: LGTM!Also applies to: 371-374
composer.json (1)
4-4: LGTM!src/Config/Config.php (1)
17-17: LGTM!Also applies to: 27-31
src/Config/ConfigApplier.php (1)
1-107: LGTM!src/Exception/ConfigException.php (1)
1-24: LGTM!src/Exception/Message.php (1)
44-66: LGTM!Also applies to: 81-87
src/Base/BaseTag.php (1)
8-15: LGTM!Also applies to: 226-230, 248-250
tests/Base/BaseTagTest.php (1)
25-49: LGTM!Also applies to: 77-112, 114-120, 122-144
tests/Config/ConfigTest.php (1)
13-13: LGTM!Also applies to: 80-108, 121-143
tests/Support/Stub/ConfigApplier.php (1)
14-18: LGTM!Also applies to: 38-38
src/Config/ConfigApplierInterface.php (1)
55-56: LGTM!src/Config/Recipe.php (1)
15-16: LGTM!src/Theme/ThemeInterface.php (1)
34-34: LGTM!tests/Config/ConfigApplierTest.php (1)
1-337: LGTM!tests/Support/Stub/ConfigurableComponent.php (1)
1-67: LGTM!tests/Support/Stub/ExtendedComponent.php (1)
1-10: LGTM!
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #100 +/- ##
========================================
Coverage ? 100.00%
Complexity ? 111
========================================
Files ? 16
Lines ? 290
Branches ? 0
========================================
Hits ? 290
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Harness. |
1 participant
Pull Request