feat: add internal projection runtime and LLM-authored artifacts - #92
Merged
Conversation
Allow the two first-party target constructors on every adapter entrypoint. Cover generated target chunks in the DOM polyfill side-effects contract.
Normalize unsafe keys and keep actor-owned inspection coherent. Remove premature named projection types while preserving target constructors.
Distinguish absent projection channels from explicit empty actor-owned state.
Reject case-insensitive on-plus-letter keys during recursive validation.
Build semantic documents from descriptor-checked data allowlists before commit.
Validate preserved JSON islands and descriptor-copy actor document collections.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis change replaces registry-oriented projection planning with validated actor-owned projection documents, opaque document and speech targets, private binding and inspection runtime support, stricter adapter contracts, hardened XState lifecycle handling, extensive tests, and updated accessibility/projection documentation. ChangesDynamic projection runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Model
participant IgniteTools
participant ActorState
participant ProjectionRuntime
participant DOMOrSpeechCommitter
Model->>IgniteTools: issue upsert or patch command
IgniteTools->>ActorState: update validated ProjectionDocument
ActorState->>ProjectionRuntime: publish inspection state
ProjectionRuntime->>DOMOrSpeechCommitter: commit document or acknowledge speech
DOMOrSpeechCommitter-->>ProjectionRuntime: return commit result
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## beta #92 +/- ##
==========================================
- Coverage 91.25% 89.97% -1.29%
==========================================
Files 33 38 +5
Lines 1830 3052 +1222
Branches 498 910 +412
==========================================
+ Hits 1670 2746 +1076
- Misses 150 284 +134
- Partials 10 22 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@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: 3
🧹 Nitpick comments (3)
packages/ignite-element/src/runtime/projectionTargets.ts (1)
84-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared brand/freeze helper to remove duplication.
createProjectionDocumentTarget(Lines 84-134) andcreateProjectionSpeechTarget(Lines 136-183) both build an initial object literal with the brand key, then immediately redefine the same properties viaObject.definePropertiesbefore freezing and registering in theWeakMap. The two-step construction is duplicated near-verbatim across both factories.♻️ Suggested consolidation
+function brandProjectionTarget<T extends Record<string, unknown>>( + kind: T["kind"], + extra: Omit<T, "kind" | typeof igniteProjectionTargetBrand>, +): T { + const target = {} as T; + Object.defineProperties(target, { + [igniteProjectionTargetBrand]: { + value: true, + enumerable: false, + writable: false, + configurable: false, + }, + kind: { value: kind, enumerable: false, writable: false, configurable: false }, + ...Object.fromEntries( + Object.entries(extra).map(([key, value]) => [ + key, + { value, enumerable: false, writable: false, configurable: false }, + ]), + ), + }); + return Object.freeze(target); +}Both factories can then call this helper instead of repeating the object-literal-then-defineProperties pattern.
🤖 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 `@packages/ignite-element/src/runtime/projectionTargets.ts` around lines 84 - 183, Extract a shared helper for constructing projection target shells with the non-enumerable, immutable igniteProjectionTargetBrand and kind properties, then freeze the result. Update createProjectionDocumentTarget and createProjectionSpeechTarget to call this helper and remove their duplicated object-literal/Object.defineProperties setup while preserving documentId handling and WeakMap registration.packages/ignite-element/src/IgniteElementFactory.ts (2)
118-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the command schema helpers
These helpers mirrorpackages/ignite-element/src/runtime/agent.ts, including thegated: truebranch. Move them into a shared internal module, such asruntime/commands.ts, so the registration and inspection schemas stay in sync.🤖 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 `@packages/ignite-element/src/IgniteElementFactory.ts` around lines 118 - 208, Move getCommandMetadata, hasCanExecute, getCommandContract, getAdditionalArg, createCommandSchemaEntry, and getOwnCommandEntries from IgniteElementFactory.ts into a shared internal runtime/commands.ts module, then update both registration and inspection code to import and use them so command schemas, including gated: true handling, remain synchronized.
906-950: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache projection inspection schema generation
resolveProjectionInspection()runs on every active projection commit, but the derivedrevisionisn’t consumed by the binding flow. Consider caching the command/schema serialization here so repeated snapshot updates don’t remap/sort/stringify every command on a hot path.🤖 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 `@packages/ignite-element/src/IgniteElementFactory.ts` around lines 906 - 950, Cache the derived projection inspection schema and revision within resolveProjectionInspection() so repeated active projection commits reuse unchanged command mappings, sorting, and serialization. Invalidate or recompute the cache only when command entries, snapshot, or view data change, while preserving canExecute, documents, and speech behavior.
🤖 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 @.fas/TASKS.md:
- Around line 2179-2189: The task status is inconsistent between TASKS.md and
the live queue. Locate the task block for “Implement internal dynamic projection
pipeline and LLM-authored artifacts” and update its Status to match
task-1783650880370 in .fas/queue/tasks.json, or complete the queue entry with
the required completedAt if the work is actually finished; ensure downstream
gating references the same authoritative state.
In `@packages/ignite-element/src/IgniteElementFactory.ts`:
- Around line 969-1039: Update commitProjectionDocumentTarget in
internal/projectionBinding.ts so documentRevisionById records the current
document revision before validation runs, or otherwise caches revisions that
fail validation. Preserve deduplication for both successful and failed
revisions, preventing repeated validation and error logging on subsequent
snapshot changes until the document revision changes.
In `@packages/ignite-element/src/internal/projectionBinding.ts`:
- Around line 102-117: Address the unused document identity contract between
createProjectionDocument() and commitProjectionDocumentTarget(): either update
commitProjectionDocumentTarget() to invoke projection.identity(document)
consistently with speech projections, or remove the identity() method from the
document projection API and related types/usages.
---
Nitpick comments:
In `@packages/ignite-element/src/IgniteElementFactory.ts`:
- Around line 118-208: Move getCommandMetadata, hasCanExecute,
getCommandContract, getAdditionalArg, createCommandSchemaEntry, and
getOwnCommandEntries from IgniteElementFactory.ts into a shared internal
runtime/commands.ts module, then update both registration and inspection code to
import and use them so command schemas, including gated: true handling, remain
synchronized.
- Around line 906-950: Cache the derived projection inspection schema and
revision within resolveProjectionInspection() so repeated active projection
commits reuse unchanged command mappings, sorting, and serialization. Invalidate
or recompute the cache only when command entries, snapshot, or view data change,
while preserving canExecute, documents, and speech behavior.
In `@packages/ignite-element/src/runtime/projectionTargets.ts`:
- Around line 84-183: Extract a shared helper for constructing projection target
shells with the non-enumerable, immutable igniteProjectionTargetBrand and kind
properties, then freeze the result. Update createProjectionDocumentTarget and
createProjectionSpeechTarget to call this helper and remove their duplicated
object-literal/Object.defineProperties setup while preserving documentId
handling and WeakMap registration.
🪄 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
Run ID: 95967063-7424-4f96-a70e-06fba3382721
📒 Files selected for processing (44)
.fas/TASKS.md.fas/queue/tasks.json.fas/tasks/add-dom-accessibility-verification-for-ignite-examples.md.fas/tasks/add-headless-accessibility-contract-assertions-to-the-test-d.md.fas/tasks/build-voice-text-agent-control-center-workbench-example-for-.md.fas/tasks/design-the-ignite-element-accessibility-contract-api-and-dx.md.fas/tasks/document-accessibility-first-ignite-implementation-patterns.md.fas/tasks/implement-internal-dynamic-projection-pipeline-and-llm-autho.md.fas/tasks/implement-the-core-accessibility-semantics-projection-surfac.mddocs/accessibility-by-default.mddocs/projection-runtime.mddocs/site/src/content/docs/guides/accessibility-first.mdxpackages/ignite-adapters/src/adapters/XStateAdapter.tspackages/ignite-core/src/utils/failInvariant.tspackages/ignite-element/package.jsonpackages/ignite-element/scripts/verify-exports.mjspackages/ignite-element/src/IgniteElementFactory.tspackages/ignite-element/src/actor-web.tspackages/ignite-element/src/createComponentFactory.tspackages/ignite-element/src/createProjectionFactory.tspackages/ignite-element/src/igniteCore/createIgniteComponentFactory.tspackages/ignite-element/src/igniteCore/redux.tspackages/ignite-element/src/igniteCore/types.tspackages/ignite-element/src/index.tspackages/ignite-element/src/internal/projectionBinding.tspackages/ignite-element/src/internal/projectionDocument.tspackages/ignite-element/src/mobx.tspackages/ignite-element/src/redux.tspackages/ignite-element/src/runtime/agent.tspackages/ignite-element/src/runtime/projectionTargets.tspackages/ignite-element/src/runtime/schema.tspackages/ignite-element/src/tests/IgniteElementFactory.test.tspackages/ignite-element/src/tests/adapters/XStateAdapter.test.tspackages/ignite-element/src/tests/createComponentFactory.test.tspackages/ignite-element/src/tests/createProjectionFactory.test.tspackages/ignite-element/src/tests/projection-binding.test.tspackages/ignite-element/src/tests/projection-runtime.test.tspackages/ignite-element/src/tests/projection-target-guard.test.tspackages/ignite-element/src/tests/tools.test.tspackages/ignite-element/src/tests/types/igniteCore.types.test.tspackages/ignite-element/src/tools/core.tspackages/ignite-element/src/types/agent.tspackages/ignite-element/src/types/projectionTargetBrand.tspackages/ignite-element/src/xstate.ts
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
.fas/scripts/verify.sh --fullpnpm --filter ignite-element test -- projection-runtime.test.ts(622 tests)coderabbit review --agent -t committed --base beta -c AGENTS.mdfas batch close --dry-runfas batch closeFollow-up boundary
The canonical prompt/speech control-center example and its final docs remain in the existing dependent workbench and documentation tasks; this PR ships the runtime contract those tasks require.
Summary by CodeRabbit