Conversation
- element->string typed as 'respo.schema/Element
- make-string typed as 'respo.schema/Component
- Added map-to-record-test: passes {} map to element->string,
verifying automatic map-to-record rewrite works cross-namespace
…ctions - Define DomProps struct in respo.schema with 29 typed fields: strings (class-name, id, href, src, etc.), bools (disabled, checked, etc.), fns (on-click, on-input, etc.), maps (style, on, event), dynamic (value, inner-text) - Type all DOM element functions (div, input, span, button, etc.) props param as (:: :optional 'respo.schema/DomProps) - Update create-element and create-list-element to convert record props back to map via &record:to-map for pick-attrs/pick-event compatibility - Map literals at call sites with only valid DomProps keys are automatically rewritten to DomProps records by Calcit's map-to-record preprocessor
…om DomProps, update props type to DomProps
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBumps snapshot version and introduces a typed dispatch Op and public schema entities (Op, DomProps, EventHandler, RespoEvent, Element, *dispatch-op). Routes dispatch calls through the Op-typed dispatcher, tightens element/DOM/event schemas, normalizes record props, and updates tests and deps. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as "Component"
participant Dispatcher as "d! macro / Dispatcher"
participant Schema as "respo.app.schema (Op)"
participant App as "App runtime"
participant Renderer as "Renderer/DOM"
Component->>Dispatcher: d! $ %:: Op :action payload
Dispatcher->>Schema: tag/validate as Op
Schema-->>Dispatcher: typed Op
Dispatcher->>App: deliver typed Op
App->>Renderer: update state / produce Element
Renderer->>Component: render / bind events
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compact.cirru`:
- Around line 2842-2843: The schema for text-width currently specifies a return
of :number but the implementation of text-width can return nil when
shared-canvas-context is falsy; update the text-width function schema to allow
nil (e.g., change :return :number to allow :nil as well or use an or/union type)
so the declared schema matches the implementation and callers can expect a
possible nil result; locate the text-width schema declaration and modify its
:return to permit nil alongside number.
- Line 1837: The schema for the parameter coord is too narrow—it's declared as
(:: :list :number) but is populated with strings/keys and numbers (e.g., conj
coord (&record:get new-tree :name) and conj coord k); update the type of coord
in the affected functions to accept a heterogeneous list (e.g., a union of
string and number or a generic any/mixed type) so it matches actual usage;
locate the functions that declare the coord argument (the ones that call conj
coord with k and with (&record:get new-tree :name)) and change their signature
from (:: :list :number) to something like (:: :list (U :string :number)) or an
equivalent broader schema type everywhere it appears (also update the same
change at the other occurrence noted around line 2252).
- Around line 945-948: The schemas for the validation functions are wrong:
update confirm-child to allow and return nil, Element, or Component (include the
Component type in its :return/schema to match its actual behavior) and change
confirm-child-pair's return schema to a pair/list type (not Element) that
represents the two-item pair it returns; edit the schema annotations on the
confirm-child and confirm-child-pair definitions so their declared :return types
match the actual returned values (include the Component symbol in confirm-child
and a pair/list type for confirm-child-pair) to fix validation mismatches.
🪄 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: 8223165c-64a0-4b1d-9fdf-03d2c9607cb9
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
compact.cirrudeps.cirrupackage.jsontest/examples/quote.html
| {} | ||
| :args $ [] :dynamic | ||
| :return $ :: :optional 'respo.schema/Element | ||
| |confirm-child-pair $ %{} :CodeEntry (:doc |) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify actual returned values vs schema declarations.
rg -n -C3 'defn confirm-child|defn confirm-child-pair|, x|, pair|:return' compact.cirruRepository: Respo/respo.calcit
Length of output: 50376
Return type schemas are incorrect for validation functions.
confirm-child returns the input value after validating it is nil, element, or component—but the schema only allows Element | nil, missing the component case. confirm-child-pair returns a list pair, not an Element.
Schema fixes needed
|confirm-child $ %{} :CodeEntry (:doc "|Validates if the item is a valid Respo node (element, component, or nil). Returns the item.")
:schema $ :: :fn
{}
:args $ [] :dynamic
- :return $ :: :optional 'respo.schema/Element
+ :return $ :: :optional :dynamic
...
|confirm-child-pair $ %{} :CodeEntry (:doc |)
:schema $ :: :fn
{}
:args $ [] :dynamic
- :return $ :: :optional 'respo.schema/Element
+ :return $ :list🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compact.cirru` around lines 945 - 948, The schemas for the validation
functions are wrong: update confirm-child to allow and return nil, Element, or
Component (include the Component type in its :return/schema to match its actual
behavior) and change confirm-child-pair's return schema to a pair/list type (not
Element) that represents the two-item pair it returns; edit the schema
annotations on the confirm-child and confirm-child-pair definitions so their
declared :return types match the actual returned values (include the Component
symbol in confirm-child and a pair/list type for confirm-child-pair) to fix
validation mismatches.
| :schema $ :: :fn | ||
| {} (:return :unit) | ||
| :args $ [] :fn :list :list :dynamic :dynamic | ||
| :args $ [] :fn (:: :list :number) (:: :list :number) :dynamic :dynamic |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify coord composition includes non-number values.
rg -n -C2 'defn find-element-diffs|conj coord|:set-event|defn add-event' compact.cirruRepository: Respo/respo.calcit
Length of output: 7058
🏁 Script executed:
sed -n '1830,1850p' compact.cirruRepository: Respo/respo.calcit
Length of output: 1270
🏁 Script executed:
sed -n '2245,2265p' compact.cirruRepository: Respo/respo.calcit
Length of output: 1070
🏁 Script executed:
# Look for the full find-element-diffs schema definition
sed -n '1832,1845p' compact.cirruRepository: Respo/respo.calcit
Length of output: 891
🏁 Script executed:
# Look for the full add-event schema definition
sed -n '2240,2260p' compact.cirruRepository: Respo/respo.calcit
Length of output: 1055
coord is typed as (:: :list :number) but carries semantic values—component names, keys, and indices.
The coord parameter in both functions is constructed with non-numeric values (component names, child keys) in addition to numeric indices. For example, at line 1780: conj coord (&record:get new-tree :name) appends a component name (string), and at line 1697: conj coord k appends keys. The current schema constraint is too strict and inconsistent with actual usage.
Proposed schema fix
|find-element-diffs
:schema $ :: :fn
{} (:return :unit)
- :args $ [] :fn (:: :list :number) (:: :list :number) :dynamic :dynamic
+ :args $ [] :fn :list (:: :list :number) :dynamic :dynamic
...
|add-event
:schema $ :: :fn
{} (:return :unit)
- :args $ [] :dynamic :tag :fn (:: :list :number)
+ :args $ [] :dynamic :tag :fn :listAlso applies to: 2252-2252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compact.cirru` at line 1837, The schema for the parameter coord is too
narrow—it's declared as (:: :list :number) but is populated with strings/keys
and numbers (e.g., conj coord (&record:get new-tree :name) and conj coord k);
update the type of coord in the affected functions to accept a heterogeneous
list (e.g., a union of string and number or a generic any/mixed type) so it
matches actual usage; locate the functions that declare the coord argument (the
ones that call conj coord with k and with (&record:get new-tree :name)) and
change their signature from (:: :list :number) to something like (:: :list (U
:string :number)) or an equivalent broader schema type everywhere it appears
(also update the same change at the other occurrence noted around line 2252).
| {} (:return :number) | ||
| :args $ [] :string :number :string |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify nil-return branch in text-width.
rg -n -C3 'defn text-width|shared-canvas-context|, nil|:return' compact.cirruRepository: Respo/respo.calcit
Length of output: 50376
🏁 Script executed:
sed -n '2825,2850p' compact.cirruRepository: Respo/respo.calcit
Length of output: 1323
text-width schema should allow nil return.
Implementation returns nil when shared-canvas-context is falsy (when canvas operations are unavailable), but schema currently declares :return :number only.
Proposed schema fix
|text-width
:schema $ :: :fn
- {} (:return :number)
+ {} (:return (:: :optional :number))
:args $ [] :string :number :string📝 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.
| {} (:return :number) | |
| :args $ [] :string :number :string | |
| {} (:return (:: :optional :number)) | |
| :args $ [] :string :number :string |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compact.cirru` around lines 2842 - 2843, The schema for text-width currently
specifies a return of :number but the implementation of text-width can return
nil when shared-canvas-context is falsy; update the text-width function schema
to allow nil (e.g., change :return :number to allow :nil as well or use an
or/union type) so the declared schema matches the implementation and callers can
expect a possible nil result; locate the text-width schema declaration and
modify its :return to permit nil alongside number.
There was a problem hiding this comment.
Pull request overview
Updates the project to a newer @calcit/procs / Calcit version and refreshes the generated snapshot to reflect improved/stricter schema typing, along with small SSR fixture/test adjustments.
Changes:
- Bump
@calcit/procsand:calcit-versionfrom0.12.14to0.12.15. - Update generated
compact.cirrusnapshot with new/updated schema types (e.g.,DomProps,RespoEvent, event handler typing) and some related logic tweaks. - Update the HTML quote fixture and corresponding test to use
data-nameinstead of a custom attribute.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Locks @calcit/[email protected] resolution/checksum and updates workspace dep reference. |
| package.json | Bumps @calcit/procs dependency to 0.12.15. |
| deps.cirru | Updates :calcit-version to 0.12.15. |
| compact.cirru | Regenerated snapshot: schema/type improvements, record→map handling for props, updated tests, and new structs (DomProps, RespoEvent). |
| test/examples/quote.html | Updates expected SSR output attributes for the quote test case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| |pick-attrs $ %{} :CodeEntry (:doc "|Extracts HTML attributes from a properties map, filtering out internal keys like :on, :event, :style.") | ||
| :code $ quote | ||
| defn pick-attrs (props) | ||
| if (nil? props) ([]) | ||
| -> props (&map:dissoc :on) (&map:dissoc :event) (&map:dissoc :style) (&map:to-list) | ||
| -> props (&map:dissoc :on) (&map:dissoc :style) (&map:to-list) | ||
| filter $ fn (pair) |
There was a problem hiding this comment.
pick-attrs no longer removes :event from the props map, but the docstring still states that :event is filtered out. If callers pass :event (or legacy code still does), it will now be serialized as a normal HTML attribute during SSR (event="..."), and it also won't be consumed by pick-event. Consider adding (&map:dissoc :event) back (and/or updating the doc and tests) to prevent leaking internal event data into attributes.
| :args $ [] :dynamic | ||
| {} | ||
| :args $ [] (:: :optional :string) | ||
| :return $ :: :optional :string |
There was a problem hiding this comment.
The updated schema for escape-html declares an optional string return type, but the implementation returns a string in all branches (it returns an empty string when text is nil). This makes the new types misleading for callers; either return nil on nil input or change the schema return type to :string.
| :return $ :: :optional :string | |
| :return :string |
| defstruct DomProps | ||
| :class-name $ :: :optional :string | ||
| :style $ :: :optional :map | ||
| :value $ :: :optional :dynamic | ||
| :inner-text $ :: :optional :dynamic | ||
| :id $ :: :optional :string | ||
| :type $ :: :optional :string | ||
| :href $ :: :optional :string | ||
| :src $ :: :optional :string | ||
| :placeholder $ :: :optional :string | ||
| :name $ :: :optional :string | ||
| :title $ :: :optional :string | ||
| :disabled $ :: :optional :bool | ||
| :checked $ :: :optional :bool | ||
| :spell-check $ :: :optional :bool | ||
| :tab-index $ :: :optional :number | ||
| :read-only $ :: :optional :bool | ||
| :data-name $ :: :optional :string |
There was a problem hiding this comment.
DomProps includes :spell-check, but SSR uses prop->attr which currently does not map spell-check to the standard HTML attribute name spellcheck (unlike the existing class-name, tab-index, read-only mappings). This will produce non-standard markup in element->string/make-string. Consider adding a spell-check -> spellcheck mapping in prop->attr or renaming the prop key if you want SSR and DOM property behavior to align.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compact.cirru (1)
1276-1286:⚠️ Potential issue | 🟠 Major
prestill rejects record-backed props.Line 1277 asserts
propsisoptional :map, so a caller using the newDomPropsform will fail beforecreate-elementcan normalize it. This constructor needs the same widening as the other element helpers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 1276 - 1286, The pre element helper currently asserts props as optional :map and thus rejects record-backed DomProps; update the assertion in the defn pre function so props accepts the same widened type used by the other element helpers (i.e., allow either a plain map or the respo.schema/DomProps record form) before calling create-element, ensuring compatibility with the DomProps normalization path.
♻️ Duplicate comments (2)
compact.cirru (2)
944-961:⚠️ Potential issue | 🟠 MajorThese return schemas are still too narrow.
confirm-childreturnsxafter accepting nil, elements, or components, andconfirm-child-pairreturns the original two-item pair. Lines 947 and 961 still declareoptional Element, so the tightened types remain inconsistent with the implementations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 944 - 961, The return schemas for confirm-child and confirm-child-pair are too narrow; update the declared :return types to match their actual returns: for confirm-child (function name confirm-child) allow nil OR 'respo.schema/Element OR the component type returned by component? (a union of nil, Element, and Component), and for confirm-child-pair (function name confirm-child-pair) declare the :return as the original two-item pair type (e.g., a vector/list of two where the second element is nil|Element|Component) instead of :: :optional 'respo.schema/Element so the schema matches the implementation that returns the pair unchanged.
2842-2843:⚠️ Potential issue | 🟡 Minor
text-widthstill needs an optional return type.Line 2839 returns
nilwhenshared-canvas-contextis unavailable, so:return :numberis still incorrect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2842 - 2843, The return annotation for text-width is incorrect because text-width can return nil when shared-canvas-context is unavailable; update the function signature for text-width to allow an optional/nullable number return (e.g., change :return :number to an optional/nullable number type) and ensure callers handle nil; reference the text-width function and the shared-canvas-context check so the signature and any related type annotations reflect "number or nil".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compact.cirru`:
- Around line 2220-2222: The schema change at :schema $ :: :fn ... :args $ []
:map is invalid because style->html is still receiving the sorted pair list
produced by create-element and create-list-element via element->string; update
the pipeline so style->html receives a map: either modify element->string to
convert the sorted pair list into a map before calling style->html, or change
create-element/create-list-element to store :style as a map instead of a pair
list; locate the functions style->html, create-element, create-list-element, and
element->string and implement the conversion (pair list -> map) at one of those
boundaries so the serializer matches the schema.
- Around line 733-735: The listener/handler type was tightened to a single-arity
`(fn tuple -> unit)` but the dispatcher uses two-arity callbacks `(fn (op ?
data))` via wrap-dispatch and calls like `dispatch! :add |empty` (see
wrap-dispatch and try-test!/dispatch! usage); revert or relax the
listener/handler contract to accept the two-argument form by updating the
signature/type for listeners/handlers (and CodeEntry where it enforces `:args $
[] :tuple`) to allow `(fn $ {} (:return :unit) :args $ [op data] :tuple)` or an
equivalent union of one- and two-arity forms so wrapped dispatch functions
(wrap-dispatch) and callers (try-test!, dispatch!) receive the expected `(op,
data)` parameters.
- Around line 881-882: The type annotation for the key parameter `k` in the `>>`
function is too narrow (currently `:tag`); update `>>` so `k` accepts a broader
key type (e.g., a union of `:tag` and numeric ids or a generic key type) to
cover uses with task ids and op ids (see usages in
`respo.test.html/todolist-store` and `respo.app.updater`); locate the `>>`
definition and replace the `:tag` constraint on `k` with a more general type (or
remove the strict `:tag` annotation) so numeric keys and generated op ids are
valid.
- Around line 1251-1252: The atom *dispatch-fn is annotated with the wrong ref
payload (:: :ref :map); update its type to match the actual dispatch function
stored by render! (a function ref) so mount-app!, rerender-app! and
wrap-dispatch are compatible. Locate the definitions/usages of *dispatch-fn,
render!, wrap-dispatch, mount-app! and rerender-app! and replace the (:: :ref
:map) annotation with the correct function/ref type used for dispatch (e.g. a
ref-to-function signature consistent with render!'s stored value) so deref+call
in wrap-dispatch typechecks against mount-app!/rerender-app! callers. Ensure you
update the same incorrect annotation at the other occurrence noted (lines
referenced in the comment).
- Around line 2184-2186: The schema for make-string is too narrow — it declares
its argument as only 'respo.schema/Component but callers like html-quote-test,
nil-prop-test, and textarea-test pass elements; update make-string's
schema/signature (the :schema for make-string) to accept both components and
element nodes (e.g., a union type that includes the element shape or a more
general node type) so the serializer API matches actual callers (keep
make-string's :return as string but broaden the :args type instead).
- Around line 1013-1014: The schema for create-list-element incorrectly types
the children as :map even though create-list-element iterates child-map as [key
child] pairs and callers supply a pair-list via .to-list/map and list->; update
the type annotation used in create-list-element (and the similar annotation at
the other site) to accept the pair-list/sequence-of-pairs shape (e.g. a
list/sequence of [key child] pairs or the specific pair-list schema your
codebase uses) instead of a plain :map so list-> inputs validate correctly;
refer to create-list-element and child-map (and callers using .to-list, map,
list->) when making the change.
---
Outside diff comments:
In `@compact.cirru`:
- Around line 1276-1286: The pre element helper currently asserts props as
optional :map and thus rejects record-backed DomProps; update the assertion in
the defn pre function so props accepts the same widened type used by the other
element helpers (i.e., allow either a plain map or the respo.schema/DomProps
record form) before calling create-element, ensuring compatibility with the
DomProps normalization path.
---
Duplicate comments:
In `@compact.cirru`:
- Around line 944-961: The return schemas for confirm-child and
confirm-child-pair are too narrow; update the declared :return types to match
their actual returns: for confirm-child (function name confirm-child) allow nil
OR 'respo.schema/Element OR the component type returned by component? (a union
of nil, Element, and Component), and for confirm-child-pair (function name
confirm-child-pair) declare the :return as the original two-item pair type
(e.g., a vector/list of two where the second element is nil|Element|Component)
instead of :: :optional 'respo.schema/Element so the schema matches the
implementation that returns the pair unchanged.
- Around line 2842-2843: The return annotation for text-width is incorrect
because text-width can return nil when shared-canvas-context is unavailable;
update the function signature for text-width to allow an optional/nullable
number return (e.g., change :return :number to an optional/nullable number type)
and ensure callers handle nil; reference the text-width function and the
shared-canvas-context check so the signature and any related type annotations
reflect "number or nil".
🪄 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: c1591c79-fec1-4946-b98e-018d0d2f716e
📒 Files selected for processing (2)
compact.cirrutest/examples/quote.html
| {} (:return 'respo.schema/Element) | ||
| :args $ [] :tag (:: :optional :map) :map | ||
| :args $ [] :tag (:: :optional 'respo.schema/DomProps) :map |
There was a problem hiding this comment.
list-> still consumes a pair list, not a map.
create-list-element iterates child-map as [key child] pairs, and the in-file callers build it with .to-list/map. Typing both APIs as :map will reject the normal list-> input shape.
Suggested schema fix
- :args $ [] :tag (:: :optional 'respo.schema/DomProps) :map
+ :args $ [] :tag (:: :optional 'respo.schema/DomProps) :list
...
- :args $ [] (:: :optional 'respo.schema/DomProps) :map
+ :args $ [] (:: :optional 'respo.schema/DomProps) :listAlso applies to: 1232-1233
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compact.cirru` around lines 1013 - 1014, The schema for create-list-element
incorrectly types the children as :map even though create-list-element iterates
child-map as [key child] pairs and callers supply a pair-list via .to-list/map
and list->; update the type annotation used in create-list-element (and the
similar annotation at the other site) to accept the pair-list/sequence-of-pairs
shape (e.g. a list/sequence of [key child] pairs or the specific pair-list
schema your codebase uses) instead of a plain :map so list-> inputs validate
correctly; refer to create-list-element and child-map (and callers using
.to-list, map, list->) when making the change.
| :schema $ :: :fn | ||
| {} (:return :string) | ||
| :args $ [] :dynamic | ||
| :args $ [] 'respo.schema/Component |
There was a problem hiding this comment.
make-string cannot be component-only.
html-quote-test, nil-prop-test, and textarea-test all pass elements into make-string. Line 2186 narrows the serializer API past its actual callers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compact.cirru` around lines 2184 - 2186, The schema for make-string is too
narrow — it declares its argument as only 'respo.schema/Component but callers
like html-quote-test, nil-prop-test, and textarea-test pass elements; update
make-string's schema/signature (the :schema for make-string) to accept both
components and element nodes (e.g., a union type that includes the element shape
or a more general node type) so the serializer API matches actual callers (keep
make-string's :return as string but broaden the :args type instead).
- Add deftype-slot *dispatch-op in respo.schema - Update EventHandler schema to reference *dispatch-op slot - Add bind-type :dispatch-op Op in main! - Convert d! calls from generic :: to typed %:: Op construction - Import Op in comp namespaces that use d!
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compact.cirru (1)
1282-1292:⚠️ Potential issue | 🟠 Major
preis now the odd element helper that rejectsDomPropsrecords.Line 1283 still asserts
propsis:map, soprefails beforecreate-elementcan run the new record-to-map normalization. Every other helper on this path accepts the new prop type.Keep `pre` aligned with the other wrappers
defn pre (props & children) - assert-type props $ :: :optional :map create-element :pre props & children🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 1282 - 1292, The helper function pre currently asserts props is :: :optional :map which causes it to reject DomProps records before the record-to-map normalization runs; update pre so it accepts the same prop types as other helpers by widening the assertion to allow DomProps records (or remove the strict :map assertion and perform record->map normalization immediately before calling create-element), ensuring the code path still calls create-element :pre props & children after normalization; reference the pre function, its assert-type props check, and the create-element :pre invocation when making the change.
♻️ Duplicate comments (3)
compact.cirru (3)
2191-2193:⚠️ Potential issue | 🟠 Major
make-stringis still narrower than its callers.
html-quote-test,nil-prop-test, andtextarea-testall pass elements here, but Line 2193 constrains the API toComponentonly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2191 - 2193, The make-string function's type signature is too narrow (it currently annotates :args as respo.schema/Component) but callers like html-quote-test, nil-prop-test, and textarea-test pass elements; update the make-string declaration to accept the broader element type used by those callers (e.g., change the :args annotation on make-string from respo.schema/Component to the appropriate element type or a union/any type that covers both Component and Element), adjust any related return/type annotations if needed, and ensure references to make-string in the code still type-check with the new signature.
1001-1020:⚠️ Potential issue | 🟠 Major
create-list-elementstill declares the wrong children shape.Line 1012 iterates
child-mapas[key child]pairs, and in-file callers build that value with.to-list/map, not a plain map. Keeping Line 1020 at:mapwill reject normallist->input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 1001 - 1020, The schema for create-list-element declares the children argument as :map but callers pass a list of [key child] pairs (built via .to-list/map); update the function schema in create-list-element to expect a list (change the final arg spec from :map to :list or the appropriate respo.schema/ChildList type) so it accepts the list-of-pairs shape that confirm-child-pair handles, keeping confirm-child-pair and child-map usage unchanged.
2508-2513:⚠️ Potential issue | 🟠 MajorThe typed dispatch callback still doesn't match
d!at runtime.
wrap-dispatchreturns(fn (op ? data)), and handlers in this snapshot calld!with two arguments. The newEventHandlerschema models the second parameter as a one-argument function over*dispatch-op, so typed handlers no longer match the callback they actually receive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2508 - 2513, The EventHandler typing is wrong: wrap-dispatch actually provides a two-argument callback (op, data) but EventHandler (and the type of *dispatch-op) was modeled as a one-arg function, causing a mismatch with handlers calling d! with two args. Update the schema for EventHandler/*dispatch-op to a two-argument function signature that matches wrap-dispatch (i.e., accepts op and data), or adjust the EventHandler definition to reference that two-argument dispatch type so handlers using d! (two args) type-check against the real runtime callback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compact.cirru`:
- Around line 2153-2155: The schema for element->string currently restricts its
arg to respo.schema/Element which excludes the plain map used by
map-to-record-test; update the element->string argument schema to accept the map
form (e.g., widen to a union of respo.schema/Element and the plain map shape or
a more permissive map schema) so calls from map-to-record-test succeed; locate
the element->string schema declaration and change the :args entry (and any
associated :fn spec) to include the map variant while preserving existing
Element validation.
- Around line 2513-2529: RespoEvent's :type is declared as a tag but event->edn
still leaves blur/unhandled events as raw strings (DomProps introduced
:on-blur), causing schema mismatch; update the event converter (event->edn) to
normalize blur and other unhandled DOM event names into the same tag form used
by RespoEvent (or vice‑versa) so that RespoEvent :type always matches (:type
:tag); locate the event->edn conversion logic and ensure it maps blur and
similar handlers (e.g. :on-blur) to the tag representation used in the
RespoEvent defstruct rather than leaving them as plain strings.
- Around line 430-432: The Op enum definition currently constrains :remove,
:update, and :toggle slots to :string which rejects existing numeric task ids;
update the Op declaration (the line starting with "defenum Op") to broaden those
three slot types from :string to a more permissive type (e.g., :dynamic or a
union of :string and :number) so that comp-task dispatches like (:id task)
accepting numeric ids are accepted by the typed dispatcher.
---
Outside diff comments:
In `@compact.cirru`:
- Around line 1282-1292: The helper function pre currently asserts props is ::
:optional :map which causes it to reject DomProps records before the
record-to-map normalization runs; update pre so it accepts the same prop types
as other helpers by widening the assertion to allow DomProps records (or remove
the strict :map assertion and perform record->map normalization immediately
before calling create-element), ensuring the code path still calls
create-element :pre props & children after normalization; reference the pre
function, its assert-type props check, and the create-element :pre invocation
when making the change.
---
Duplicate comments:
In `@compact.cirru`:
- Around line 2191-2193: The make-string function's type signature is too narrow
(it currently annotates :args as respo.schema/Component) but callers like
html-quote-test, nil-prop-test, and textarea-test pass elements; update the
make-string declaration to accept the broader element type used by those callers
(e.g., change the :args annotation on make-string from respo.schema/Component to
the appropriate element type or a union/any type that covers both Component and
Element), adjust any related return/type annotations if needed, and ensure
references to make-string in the code still type-check with the new signature.
- Around line 1001-1020: The schema for create-list-element declares the
children argument as :map but callers pass a list of [key child] pairs (built
via .to-list/map); update the function schema in create-list-element to expect a
list (change the final arg spec from :map to :list or the appropriate
respo.schema/ChildList type) so it accepts the list-of-pairs shape that
confirm-child-pair handles, keeping confirm-child-pair and child-map usage
unchanged.
- Around line 2508-2513: The EventHandler typing is wrong: wrap-dispatch
actually provides a two-argument callback (op, data) but EventHandler (and the
type of *dispatch-op) was modeled as a one-arg function, causing a mismatch with
handlers calling d! with two args. Update the schema for
EventHandler/*dispatch-op to a two-argument function signature that matches
wrap-dispatch (i.e., accepts op and data), or adjust the EventHandler definition
to reference that two-argument dispatch type so handlers using d! (two args)
type-check against the real runtime callback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
♻️ Duplicate comments (11)
compact.cirru (11)
430-432:⚠️ Potential issue | 🟠 MajorBroaden
Optask-id slots.
:remove,:update, and:toggleare still constrained to:string, but this repo still carries numeric task ids intodolist-storeand older persisted stores can do the same. The typed dispatcher will now reject valid existing ids.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 430 - 432, The Op enum definition restricts task-id slots :remove, :update, and :toggle to :string which rejects numeric ids from existing stores; update the CodeEntry/Op declaration to broaden those slots to accept both strings and numbers (e.g., change their types from :string to a more permissive type such as :dynamic or a union) so the dispatcher will accept numeric task ids; locate the Op declaration (symbol "Op" in the CodeEntry block) and modify the type annotations for :remove, :update, and :toggle accordingly, keeping other fields unchanged.
887-888:⚠️ Potential issue | 🟠 Major
>>still narrows keys too far.
kis typed as:tag, but cursor keys in this codebase are not tag-only; task ids can be numeric or generated ids. This annotation will reject normal>> states task-idusage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 887 - 888, The type annotation for key variable k (annotated as :tag) in the >> operator implementation is too restrictive and rejects valid cursor keys like numeric task ids; update the type for k (used in the >> handling that returns :map and in expressions involving states and task-id) to a union or broader key type (e.g., :tag|:id|:string or a generic :key) so it accepts both tag-only and numeric/generated ids, and adjust any related type checks in the >> implementation to use this broader key type instead of :tag.
2153-2155:⚠️ Potential issue | 🟠 Major
element->stringexcludes the new map-based path.
map-to-record-testnow callselement->stringwith a plain map-shaped element, but this schema still only acceptsrespo.schema/Element. The type no longer matches the new supported input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2153 - 2155, The schema for element->string currently restricts its single argument to 'respo.schema/Element, which rejects the new map-shaped element used by map-to-record-test; update the function schema for element->string to accept the new map-based shape (e.g., broaden the :args type to a union that includes the plain map representation or a more permissive type like any/map) so the test can pass and the function documents both supported input shapes.
1257-1258:⚠️ Potential issue | 🟠 Major
*dispatch-fnis still typed as the wrong ref payload.These APIs receive a ref holding the dispatch function, but the schema says
(:: :ref :map). That does not match the value stored byrender!, and it makesmount-app!/rerender-app!incompatible with the actual dispatcher path.Also applies to: 1342-1343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 1257 - 1258, The *dispatch-fn var is annotated with the wrong ref payload ((:: :ref :map)) causing type mismatch with the actual dispatch function stored by render!; update the schema/type annotation for *dispatch-fn to the correct ref payload shape that render! produces (the dispatcher function/ref type) and adjust any related annotations in mount-app! and rerender-app! to accept that same ref type so their signatures align with render!'s stored dispatcher; search for the symbols *dispatch-fn, render!, mount-app!, and rerender-app! and change the ref payload type to the correct function/ref schema used by render!.
2227-2229:⚠️ Potential issue | 🟠 Major
style->htmlis still typed as a map even though the pipeline passes a pair list.
create-element/create-list-elementkeep:styleas a sorted list of entries, andelement->stringforwards that value directly. Narrowingstyle->htmlto:mapmakes the serializer schema inconsistent with its actual input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2227 - 2229, The schema for style->html is incorrectly constrained to a :map even though create-element/create-list-element and element->string pass a sorted/pair list of style entries; update the serializer schema for style->html to accept the pair-list representation used by the pipeline (the same sorted list-of-entries shape emitted by create-element/create-list-element) instead of :map, and ensure any references in the serializer/element->string codebase match that pair-list type so the declared schema matches actual inputs.
951-953:⚠️ Potential issue | 🟠 MajorThese validator return schemas do not match the actual values returned.
confirm-childreturnsnil, an element, or a component, whileconfirm-child-pairreturns the full[key child]pair. Declaring both as optionalElementis incorrect and will mis-type valid trees.Also applies to: 965-967
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 951 - 953, The return schemas are incorrect: update the spec for confirm-child to allow nil, an Element, or a Component (e.g., a union of nil, respo.schema/Element, and respo.schema/Component) instead of only optional Element, and update confirm-child-pair to return an optional pair (a two-element vector/tuple like [key child], e.g., (s/nilable [Any respo.schema/Element]) or the project's tuple type) rather than an Element; adjust the :return annotations on the confirm-child and confirm-child-pair definitions to these correct union/tuple types so valid trees are typed correctly.
2487-2488:⚠️ Potential issue | 🟠 Major
RespoEvent.:typedoes not match blur/unhandled events.
DomPropsnow exposes:on-blur, butevent->ednstill falls through to the default branch for blur and other unhandled DOM events, leaving:typeas a string. DeclaringRespoEventwith(:type :tag)makes the new public event schema incorrect for those handlers.Also applies to: 2513-2529
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2487 - 2488, The event->edn conversion leaves :type as a string for blur and other unhandled DOM events (while DomProps now exposes :on-blur), which breaks the RespoEvent schema that expects (:type :tag); update the default branch of the event->edn function so that it normalizes event type strings into the same keyword/tag representation used elsewhere (e.g., convert the incoming event type string to a keyword/tag and assign it to :type), ensuring RespoEvent consumers (and handlers tied to :on-blur) always receive :type as the agreed tag form.
2852-2853:⚠️ Potential issue | 🟡 MinorAllow
nilintext-width's return schema.The implementation returns
nilwhenshared-canvas-contextis unavailable, so:return :numberis too strict.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2852 - 2853, The return schema for the function that declares "(:return :number)" is too strict because the implementation can return nil when shared-canvas-context is missing; update the function's return schema (the "(:return :number)" entry next to the args line " :args $ [] :string :number :string") to allow nil (e.g., change to a maybe/or that permits nil and number) and ensure any callers handle the possible nil result where shared-canvas-context is used.
1019-1020:⚠️ Potential issue | 🟠 Major
create-list-element/list->still consume a pair list, not a map.The implementation maps over
child-mapas[key child]pairs, and the in-file callers build it with.to-list/map. Tightening this to:mapmakes the typed API disagree with its actual input shape.Also applies to: 1238-1239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 1019 - 1020, The typed signature for create-list-element and list-> is incorrect: both functions (which iterate child-map as [key child] pairs) actually expect a sequence/list of key-child pairs, not a map. Fix the API by changing the type annotations for the child-map parameters in create-list-element and list-> (and the duplicate signatures at the other site) from :map to the appropriate pair-list/list-of-pairs type (or remove the :map tag), so the declared types match the runtime shape consumed by the loop over [key child].
738-740:⚠️ Potential issue | 🟠 MajorThe dispatch callback contract is still typed as one-arity.
wrap-dispatchstill returns(fn (op ? data)), and callers liketry-test!use that two-argument form. These schemas narrow the dispatcher tofn(tuple -> unit)/fn(op -> unit), which does not match what handlers actually receive at runtime.Also applies to: 1312-1315, 1323-1325, 2508-2512
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 738 - 740, The dispatch callback schema is declared as one-arity but runtime handlers are called with two arguments (op, data); update the type/signature definitions so wrap-dispatch is typed to return a two-argument function (fn(op, data) -> unit) or a variadic/tuple-accepting dispatcher, and adjust related schemas that narrow the dispatcher (references: wrap-dispatch, try-test!, dispatcher) so they accept the actual runtime form (two-arity or tuple) instead of only fn(tuple -> unit) / fn(op -> unit); make the same change for the other occurrences mentioned (around the other blocks) to keep the type contract consistent.
2191-2193:⚠️ Potential issue | 🟠 Major
make-stringis still typed too narrowly.The serializer is used with plain elements in
html-quote-test,nil-prop-test, andtextarea-test, but the schema still accepts onlyrespo.schema/Component. That breaks the existing call pattern this file already relies on.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2191 - 2193, The schema for the serializer function (the :schema entry that currently returns :string and has :args $ [] 'respo.schema/Component) is too narrow — update the argument type to accept plain elements as well as components (e.g. make the arg a union like 'respo.schema/Component | respo.schema/Element or use a more general any/object type) so the serializer used by html-quote-test, nil-prop-test, and textarea-test accepts plain elements; locate the make-string/serializer schema (the :schema $ :: :fn block) and change the :args declaration accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@compact.cirru`:
- Around line 430-432: The Op enum definition restricts task-id slots :remove,
:update, and :toggle to :string which rejects numeric ids from existing stores;
update the CodeEntry/Op declaration to broaden those slots to accept both
strings and numbers (e.g., change their types from :string to a more permissive
type such as :dynamic or a union) so the dispatcher will accept numeric task
ids; locate the Op declaration (symbol "Op" in the CodeEntry block) and modify
the type annotations for :remove, :update, and :toggle accordingly, keeping
other fields unchanged.
- Around line 887-888: The type annotation for key variable k (annotated as
:tag) in the >> operator implementation is too restrictive and rejects valid
cursor keys like numeric task ids; update the type for k (used in the >>
handling that returns :map and in expressions involving states and task-id) to a
union or broader key type (e.g., :tag|:id|:string or a generic :key) so it
accepts both tag-only and numeric/generated ids, and adjust any related type
checks in the >> implementation to use this broader key type instead of :tag.
- Around line 2153-2155: The schema for element->string currently restricts its
single argument to 'respo.schema/Element, which rejects the new map-shaped
element used by map-to-record-test; update the function schema for
element->string to accept the new map-based shape (e.g., broaden the :args type
to a union that includes the plain map representation or a more permissive type
like any/map) so the test can pass and the function documents both supported
input shapes.
- Around line 1257-1258: The *dispatch-fn var is annotated with the wrong ref
payload ((:: :ref :map)) causing type mismatch with the actual dispatch function
stored by render!; update the schema/type annotation for *dispatch-fn to the
correct ref payload shape that render! produces (the dispatcher function/ref
type) and adjust any related annotations in mount-app! and rerender-app! to
accept that same ref type so their signatures align with render!'s stored
dispatcher; search for the symbols *dispatch-fn, render!, mount-app!, and
rerender-app! and change the ref payload type to the correct function/ref schema
used by render!.
- Around line 2227-2229: The schema for style->html is incorrectly constrained
to a :map even though create-element/create-list-element and element->string
pass a sorted/pair list of style entries; update the serializer schema for
style->html to accept the pair-list representation used by the pipeline (the
same sorted list-of-entries shape emitted by create-element/create-list-element)
instead of :map, and ensure any references in the serializer/element->string
codebase match that pair-list type so the declared schema matches actual inputs.
- Around line 951-953: The return schemas are incorrect: update the spec for
confirm-child to allow nil, an Element, or a Component (e.g., a union of nil,
respo.schema/Element, and respo.schema/Component) instead of only optional
Element, and update confirm-child-pair to return an optional pair (a two-element
vector/tuple like [key child], e.g., (s/nilable [Any respo.schema/Element]) or
the project's tuple type) rather than an Element; adjust the :return annotations
on the confirm-child and confirm-child-pair definitions to these correct
union/tuple types so valid trees are typed correctly.
- Around line 2487-2488: The event->edn conversion leaves :type as a string for
blur and other unhandled DOM events (while DomProps now exposes :on-blur), which
breaks the RespoEvent schema that expects (:type :tag); update the default
branch of the event->edn function so that it normalizes event type strings into
the same keyword/tag representation used elsewhere (e.g., convert the incoming
event type string to a keyword/tag and assign it to :type), ensuring RespoEvent
consumers (and handlers tied to :on-blur) always receive :type as the agreed tag
form.
- Around line 2852-2853: The return schema for the function that declares
"(:return :number)" is too strict because the implementation can return nil when
shared-canvas-context is missing; update the function's return schema (the
"(:return :number)" entry next to the args line " :args $ [] :string :number
:string") to allow nil (e.g., change to a maybe/or that permits nil and number)
and ensure any callers handle the possible nil result where
shared-canvas-context is used.
- Around line 1019-1020: The typed signature for create-list-element and list->
is incorrect: both functions (which iterate child-map as [key child] pairs)
actually expect a sequence/list of key-child pairs, not a map. Fix the API by
changing the type annotations for the child-map parameters in
create-list-element and list-> (and the duplicate signatures at the other site)
from :map to the appropriate pair-list/list-of-pairs type (or remove the :map
tag), so the declared types match the runtime shape consumed by the loop over
[key child].
- Around line 738-740: The dispatch callback schema is declared as one-arity but
runtime handlers are called with two arguments (op, data); update the
type/signature definitions so wrap-dispatch is typed to return a two-argument
function (fn(op, data) -> unit) or a variadic/tuple-accepting dispatcher, and
adjust related schemas that narrow the dispatcher (references: wrap-dispatch,
try-test!, dispatcher) so they accept the actual runtime form (two-arity or
tuple) instead of only fn(tuple -> unit) / fn(op -> unit); make the same change
for the other occurrences mentioned (around the other blocks) to keep the type
contract consistent.
- Around line 2191-2193: The schema for the serializer function (the :schema
entry that currently returns :string and has :args $ [] 'respo.schema/Component)
is too narrow — update the argument type to accept plain elements as well as
components (e.g. make the arg a union like 'respo.schema/Component |
respo.schema/Element or use a more general any/object type) so the serializer
used by html-quote-test, nil-prop-test, and textarea-test accepts plain
elements; locate the make-string/serializer schema (the :schema $ :: :fn block)
and change the :args declaration accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b6471975-7435-4558-80fe-01b6275c1b83
📒 Files selected for processing (3)
compact.cirrudeps.cirrupackage.json
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
compact.cirru (5)
2487-2489:⚠️ Potential issue | 🟠 Major
RespoEvent.:typedoes not match blur/unhandled events yet.
DomPropsnow exposes:on-blur, butevent->ednstill leaves blur and other unhandled DOM events with a raw string:type. DeclaringRespoEventwith(:type :tag)makes those handlers type-invalid unless the converter is normalized too.Also applies to: 2513-2529
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2487 - 2489, RespoEvent's :type declaration (using (:type :tag)) is incompatible with raw string types produced by event->edn for blur/unhandled DOM events; update the event->edn converter (the function named event->edn) to normalize incoming DOM event type strings into the same :tag form used by RespoEvent (e.g., map "blur"/other raw type names to the corresponding tag/keyword) so handlers declared on DomProps (:on-blur, :on-focus, etc.) receive a type that matches RespoEvent, or alternatively broaden RespoEvent's (:type :tag) to accept both tag and raw string variants consistently across event->edn and RespoEvent.
964-967:⚠️ Potential issue | 🟠 Major
list->is still typed against the wrong child shape.
confirm-child-pairreturns a[key child]pair, andcreate-list-element/list->consume that keyed pair-list form. Keeping these schemas asElement/:mapbreaks the normallist->API that current callers build with.to-listandmap.Also applies to: 1019-1020, 1238-1239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 964 - 967, The schema currently types list-> / create-list-element as consuming/returning plain Element/:map, but confirm-child-pair produces [key child] pairs; update the schemas for the functions (list->, create-list-element, and the relevant :fn schema blocks around the shown diff and also at the other two occurrences) to expect a sequence/vector of keyed child pairs (i.e. pairs [key child] or a ::pair/KeyedChild shape) rather than a bare Element/:map so callers using .to-list and map receive the keyed-pair shape that confirm-child-pair returns.
2153-2155:⚠️ Potential issue | 🟠 MajorThe serializer types still reject their own supported inputs.
map-to-record-testnow callselement->stringwith a plain map, the HTML tests still pass Elements intomake-string, andstyle->htmlstill receives the sorted style pair list produced bycreate-element/create-list-element. These three schemas need to be widened together or the new typed surface will fail on supported serialization paths.Also applies to: 2191-2193, 2227-2229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 2153 - 2155, The serializer schema for the functions (referenced by the schema snippet and used by element->string, make-string, and style->html) is too narrow and rejects valid inputs like plain maps (used by map-to-record-test) and the sorted style pair list produced by create-element/create-list-element; widen the function arg/result schemas to accept a union of the supported shapes (e.g., respo.schema/Element OR plain map OR the style pair-list form) or a more general type so element->string, make-string, and style->html accept the actual runtime inputs; update the schema declarations that currently use ':args $ [] 'respo.schema/Element and matching return types to include these union types and adjust any helper schemas used by create-element and create-list-element accordingly.
430-432:⚠️ Potential issue | 🟠 MajorBroaden task-id and cursor-key types again.
>>is called withtask-idat Line 168, and the same ids flow throughOp.:remove,Op.:update, andOp.:toggle. Those ids are not guaranteed to be:tag/:stringonly—the repo still carries numeric ids at Lines 2671-2672—so these annotations now reject valid existing update paths.Also applies to: 887-888
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 430 - 432, The current Op enum restricts task-id/cursor-key fields to :string/:tag which breaks valid numeric ids; update the Op code entry so fields carrying task ids (specifically the :add, :remove, :update and :toggle members) use a broader type (e.g., :dynamic or a union that includes numeric ids) instead of :string, and make the same change in the other Op/code-entry occurrence noted (the second enum instance around the other mentioned lines) so all flows that call >> with task-id accept both string and numeric ids.
1257-1258:⚠️ Potential issue | 🟠 MajorRenderer dispatch schemas are still inconsistent with the stored value.
*dispatch-fnholds a function, not(:: :ref :map), and the callback passed throughrender!/realize-ssr!is the raw dispatcher later wrapped bywrap-dispatch. These annotations make the render pipeline incompatible with the actual value coming fromrespo.app.core/dispatch!.Also applies to: 1311-1314, 1321-1325, 1342-1343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compact.cirru` around lines 1257 - 1258, The renderer schema annotations wrongly expect a map (:: :ref :map) for the dispatcher but the actual value is a function; update the type annotations used around *dispatch-fn, the callback parameters to render! and realize-ssr!, and any places referencing wrap-dispatch/respo.app.core/dispatch! to reflect a function type (e.g., a Fn signature) or a union that includes functions, and ensure the schema for the argument passed into wrap-dispatch matches the raw dispatcher function rather than a map so the render pipeline and the stored dispatch! value are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compact.cirru`:
- Around line 2505-2512: The EventHandler type definition narrows the dispatcher
to only a single *dispatch-op which breaks existing handlers that call d! with
(op ? data) (used by local updates like d! cursor) and :on-* callbacks; update
the EventHandler signature (the :code/quote and :schema entries surrounding
EventHandler and respo.schema/RespoEvent) to accept both forms—either by using a
union/overload that allows the original (op ? data) arity and the new
*dispatch-op form or by making the dispatcher variadic so d! calls and typed ops
remain valid; ensure the change targets the EventHandler declaration and its
:schema :: :fn definition so existing :on-* callbacks are not rejected.
---
Duplicate comments:
In `@compact.cirru`:
- Around line 2487-2489: RespoEvent's :type declaration (using (:type :tag)) is
incompatible with raw string types produced by event->edn for blur/unhandled DOM
events; update the event->edn converter (the function named event->edn) to
normalize incoming DOM event type strings into the same :tag form used by
RespoEvent (e.g., map "blur"/other raw type names to the corresponding
tag/keyword) so handlers declared on DomProps (:on-blur, :on-focus, etc.)
receive a type that matches RespoEvent, or alternatively broaden RespoEvent's
(:type :tag) to accept both tag and raw string variants consistently across
event->edn and RespoEvent.
- Around line 964-967: The schema currently types list-> / create-list-element
as consuming/returning plain Element/:map, but confirm-child-pair produces [key
child] pairs; update the schemas for the functions (list->, create-list-element,
and the relevant :fn schema blocks around the shown diff and also at the other
two occurrences) to expect a sequence/vector of keyed child pairs (i.e. pairs
[key child] or a ::pair/KeyedChild shape) rather than a bare Element/:map so
callers using .to-list and map receive the keyed-pair shape that
confirm-child-pair returns.
- Around line 2153-2155: The serializer schema for the functions (referenced by
the schema snippet and used by element->string, make-string, and style->html) is
too narrow and rejects valid inputs like plain maps (used by map-to-record-test)
and the sorted style pair list produced by create-element/create-list-element;
widen the function arg/result schemas to accept a union of the supported shapes
(e.g., respo.schema/Element OR plain map OR the style pair-list form) or a more
general type so element->string, make-string, and style->html accept the actual
runtime inputs; update the schema declarations that currently use ':args $ []
'respo.schema/Element and matching return types to include these union types and
adjust any helper schemas used by create-element and create-list-element
accordingly.
- Around line 430-432: The current Op enum restricts task-id/cursor-key fields
to :string/:tag which breaks valid numeric ids; update the Op code entry so
fields carrying task ids (specifically the :add, :remove, :update and :toggle
members) use a broader type (e.g., :dynamic or a union that includes numeric
ids) instead of :string, and make the same change in the other Op/code-entry
occurrence noted (the second enum instance around the other mentioned lines) so
all flows that call >> with task-id accept both string and numeric ids.
- Around line 1257-1258: The renderer schema annotations wrongly expect a map
(:: :ref :map) for the dispatcher but the actual value is a function; update the
type annotations used around *dispatch-fn, the callback parameters to render!
and realize-ssr!, and any places referencing
wrap-dispatch/respo.app.core/dispatch! to reflect a function type (e.g., a Fn
signature) or a union that includes functions, and ensure the schema for the
argument passed into wrap-dispatch matches the raw dispatcher function rather
than a map so the render pipeline and the stored dispatch! value are consistent.
🪄 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: 16c44103-b41b-4ab8-95d7-2e83e08cae9b
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
compact.cirrudeps.cirrupackage.json
Summary by CodeRabbit
Version
Improvements
Tests
Chores