Conversation
|
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:
WalkthroughDocs reorganized: Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
There was a problem hiding this comment.
Pull request overview
This pull request adds comprehensive documentation for LLM/AI agents working with Calcit projects and introduces optional type hints throughout the codebase to improve code quality and enable better type checking.
Changes:
- Added extensive LLM-friendly documentation with search workflows, command examples, and best practices
- Introduced type annotations using
assert-typefor function parameters andhint-fnwithreturn-typefor return values - Updated dependencies (@calcit/procs 0.10.6 → 0.10.10, rollup 4.55.1 → 4.55.2)
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updated rollup dependencies from 4.55.1 to 4.55.2 and @calcit/procs ecosystem packages to newer versions |
| package.json | Updated @calcit/procs dependency from 0.10.6 to 0.10.10 |
| docs/Respo-Agent.md | Added comprehensive CSS styling documentation including defstyle macro usage, pseudo-classes, media queries, and SSR support |
| deps.cirru | Updated calcit-version from 0.10.6 to 0.10.10 for consistency |
| compact.cirru | Added type hints (assert-type, hint-fn) to numerous functions, updated version to 0.16.25, added warning message in :about field |
| calcit.cirru | Updated version from 0.16.24 to 0.16.25 |
| Agents.md | Added extensive LLM-specific documentation covering quick start workflows, search commands, type annotations, shell escaping, and efficiency comparisons |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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)
docs/Respo-Agent.md (1)
452-476: Use subheadings instead of bold-only section titlesThe A/B subsections are formatted as emphasis, which triggers MD036 and makes navigation harder. Convert them to heading levels under “Styling Pattern.”
🛠️ Suggested fix
-**A. Dynamic Inline Styles (Style Maps)** +#### A. Dynamic Inline Styles (Style Maps) @@ -**B. Static CSS Styles with `defstyle` (Recommended for Performance)** +#### B. Static CSS Styles with `defstyle` (Recommended for Performance)
🤖 Fix all issues with AI agents
In `@Agents.md`:
- Around line 344-347: The fenced bash code block starting with triple backticks
containing the command "cr tree replace ns/def -p '2' -e '(+ $ $)'
--refer-original '$'" violates MD031; add a blank line immediately before the
opening ```bash fence and a blank line immediately after the closing ``` fence
so there is an empty line surrounding the entire fenced block (update the block
around the shown command in Agents.md).
- Line 7: Replace the bold line "**核心原则:用命令行工具(不要直接编辑文件),用 search 定位(比逐层导航快 10
倍)**" with a proper Markdown heading (e.g., prefix with one or more #) so it
becomes a heading like "## 核心原则:用命令行工具(不要直接编辑文件),用 search 定位(比逐层导航快 10 倍)" to
satisfy markdownlint MD036; update only that line (preserve the exact text
content and punctuation, just change the Markdown from bold to heading).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@compact.cirru`:
- Line 825: The helper function defined by "defn code (props & children)
(assert-type props :map) (create-element :code props & children)" requires props
to be a map but should allow nil for consistency with other element helpers;
change the assertion to use the optional form (like other helpers) so that
"assert-type props $ :: :optional :map" is used before calling create-element
:code props & children to accept nil props without breaking existing callers.
In `@package.json`:
- Around line 3-6: The "test" npm script in package.json currently runs "cr ...
js" but then invokes node on test.mjs which doesn't match cr's default emit
path; fix the script "test" (the npm script value) so the node invocation
matches the actual emitted file by either adding the cr flag to emit to project
root (use --emit-path=.) before running node test.mjs, or update the node target
to the emitted file (node js-out/respo.test.main.mjs); change the script value
accordingly so cr's output path and the node invocation are consistent.
♻️ Duplicate comments (2)
Agents.md (2)
7-7: Convert the bold line to a heading (MD036).
344-347: Add blank lines around fenced code block (MD031).
| |code $ %{} :CodeEntry (:doc |) | ||
| :code $ quote | ||
| defn code (props & children) (create-element :code props & children) | ||
| defn code (props & children) (assert-type props :map) (create-element :code props & children) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the compact.cirru file
find . -name "compact.cirru" -type fRepository: Respo/respo.calcit
Length of output: 77
🏁 Script executed:
# Let's also check the structure of the repository
git ls-files | head -30Repository: Respo/respo.calcit
Length of output: 685
🏁 Script executed:
# Let's examine the compact.cirru file, especially around line 825
wc -l ./compact.cirruRepository: Respo/respo.calcit
Length of output: 82
🏁 Script executed:
# Read the code function and surrounding context
sed -n '820,830p' ./compact.cirruRepository: Respo/respo.calcit
Length of output: 661
🏁 Script executed:
# Now let's search for other element helper definitions to compare
# Look for patterns like "defn div", "defn span", "defn p", etc.
rg "defn \w+ \(props & children\)" ./compact.cirru -A 1Repository: Respo/respo.calcit
Length of output: 3168
Make code props optional for API consistency.
The code function currently uses assert-type props :map, while most other element helpers like div, span, p, button, input, select, textarea, a, li, ol, ul, h1, h2, and h3 use assert-type props $ :: :optional :map. This inconsistency can break existing code when nil is passed.
Suggested fix
-defn code (props & children) (assert-type props :map) (create-element :code props & children)
+defn code (props & children) (assert-type props $ :: :optional :map) (create-element :code props & children)🤖 Prompt for AI Agents
In `@compact.cirru` at line 825, The helper function defined by "defn code (props
& children) (assert-type props :map) (create-element :code props & children)"
requires props to be a map but should allow nil for consistency with other
element helpers; change the assertion to use the optional form (like other
helpers) so that "assert-type props $ :: :optional :map" is used before calling
create-element :code props & children to accept nil props without breaking
existing callers.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/Respo-Agent.md (1)
482-504: Use headings instead of bold text for subsections.Markdownlint MD036 flags bold text used as headings. Converting to headings improves structure and navigation.
💡 Suggested heading change
-**A. Dynamic Inline Styles (Style Maps)** +#### A. Dynamic Inline Styles (Style Maps) -**B. Static CSS Styles with `defstyle` (Recommended for Performance)** +#### B. Static CSS Styles with `defstyle` (Recommended for Performance)
🤖 Fix all issues with AI agents
In `@docs/Respo-Agent.md`:
- Around line 307-309: Rewrite the three consecutive lines that all start with
"Use" to vary sentence openings while preserving the commands and their
meanings: keep the command tokens `call-graph`, `search-expr`, and `query
usages` and their descriptions (WHO calls WHOM, HOW features are implemented,
WHERE a function is integrated) but change the phrasing so each line begins
differently (e.g., "Run `call-graph` to...", "Employ `search-expr` to...",
"Query `query usages` to...") to improve readability and flow.
♻️ Duplicate comments (1)
compact.cirru (1)
825-825:codeshould accept nil props for API consistency.Line 825 still asserts
props :mapwhile other element helpers accept optional maps, which can breakcodeusage without props.🐛 Suggested fix
-defn code (props & children) (assert-type props :map) (create-element :code props & children) +defn code (props & children) (assert-type props $ :: :optional :map) (create-element :code props & children)
| 1. Use `call-graph` to find **WHO** calls **WHOM**. | ||
| 2. Use `search-expr` to find **HOW** certain features are implemented project-wide. | ||
| 3. Use `query usages` to find **WHERE** a specific function is integrated. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reduce repeated sentence starts for readability.
Lines 307–309 start with “Use”. Consider varying the phrasing to improve flow.
💡 Suggested wording
-1. Use `call-graph` to find **WHO** calls **WHOM**.
-2. Use `search-expr` to find **HOW** certain features are implemented project-wide.
-3. Use `query usages` to find **WHERE** a specific function is integrated.
+1. Start with `call-graph` to find **WHO** calls **WHOM**.
+2. Apply `search-expr` to see **HOW** features are implemented project-wide.
+3. Use `query usages` to locate **WHERE** a specific function is integrated.📝 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.
| 1. Use `call-graph` to find **WHO** calls **WHOM**. | |
| 2. Use `search-expr` to find **HOW** certain features are implemented project-wide. | |
| 3. Use `query usages` to find **WHERE** a specific function is integrated. | |
| 1. Start with `call-graph` to find **WHO** calls **WHOM**. | |
| 2. Apply `search-expr` to see **HOW** features are implemented project-wide. | |
| 3. Use `query usages` to locate **WHERE** a specific function is integrated. |
🧰 Tools
🪛 LanguageTool
[style] ~309-~309: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...atures are implemented project-wide. 3. Use query usages to find WHERE a spec...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
In `@docs/Respo-Agent.md` around lines 307 - 309, Rewrite the three consecutive
lines that all start with "Use" to vary sentence openings while preserving the
commands and their meanings: keep the command tokens `call-graph`,
`search-expr`, and `query usages` and their descriptions (WHO calls WHOM, HOW
features are implemented, WHERE a function is integrated) but change the
phrasing so each line begins differently (e.g., "Run `call-graph` to...",
"Employ `search-expr` to...", "Query `query usages` to...") to improve
readability and flow.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@Agents.md`:
- Line 1: The file is missing a top-level H1 heading which triggers markdownlint
MD041; add a first-line H1 to Agents.md (for example add a line like "# Agents"
as the very first line) so the file begins with a top-level heading and
satisfies the MD041 rule.
In `@llms/Calcit.md`:
- Line 199: Replace the bolded sentence "**⚠️ 重要:生成 Cirru 代码前请先阅读语法指南**" with a
proper Markdown heading (e.g., prepend one or more # characters like "## ⚠️
重要:生成 Cirru 代码前请先阅读语法指南") so it becomes a section title; update the heading
level to match nearby headings in Calcit.md to satisfy MD036 and maintain
document structure.
- Line 7: The line currently formatted as emphasis/bold
("核心原则:用命令行工具(不要直接编辑文件),用 search 定位(比逐层导航快 10 倍)") should be promoted to a
Markdown heading to satisfy MD036; replace the emphasis with an appropriate
heading level (e.g., add "## " or "### " before that text) so the line becomes a
proper heading rather than emphasized text.
- Around line 344-347: Add an empty line before and after the fenced code block
that starts with "```bash" (the block containing the lines "# 将节点 x 变为 (+ x x)"
and "cr tree replace ns/def -p '2' -e '(+ $ $)' --refer-original '$'") so the
markdown has a blank line preceding the opening ```bash and a blank line
following the closing ```, satisfying MD031.
| @@ -1,724 +1,8 @@ | |||
| # Calcit 编程 Agent 指南 | |||
| 一般开发者会运行: | |||
There was a problem hiding this comment.
Add a top‑level heading as the first line (MD041).
Markdownlint requires a first-line H1.
🛠️ Proposed fix
-一般开发者会运行:
+# 开发者常用命令
+
+一般开发者会运行:📝 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.
| 一般开发者会运行: | |
| # 开发者常用命令 | |
| 一般开发者会运行: |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In `@Agents.md` at line 1, The file is missing a top-level H1 heading which
triggers markdownlint MD041; add a first-line H1 to Agents.md (for example add a
line like "# Agents" as the very first line) so the file begins with a top-level
heading and satisfies the MD041 rule.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@compact.cirru`:
- Around line 770-773: The defn for <> currently asserts content is a string
which breaks callers that pass keywords or non-string values; remove the
assert-type content :string and coerce content to a string inside <> (e.g., call
str on content once and use that variable for (:inner-text content) placements)
so existing usages like (<> text) or (<> |Remove) work; keep the existing style
handling logic (the branch that checks (string? style) and chooses (:class-name
style) vs (:style style)) unchanged.
In `@package.json`:
- Line 3: Replace the non-existent pre-release version of the Calcit package in
package.json by updating the "@calcit/procs" dependency from "0.11.0-a4" to a
published version (for example "0.10.11"); also ensure any companion Calcit
runtime dependency (e.g., "@calcit/runtime" or similar) uses the matching
published version "0.10.11" so the package.json references only published
versions that resolve on npm.
| defn <> (content ? style) (assert-type content :string) | ||
| if (string? style) | ||
| span $ {} (:inner-text content) (:class-name style) | ||
| span $ {} (:inner-text content) (:style style) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "compact.cirru" -type fRepository: Respo/respo.calcit
Length of output: 77
🏁 Script executed:
sed -n '765,780p' ./compact.cirruRepository: Respo/respo.calcit
Length of output: 1097
🏁 Script executed:
# Check git history for recent changes
git log --oneline -20 -- ./compact.cirruRepository: Respo/respo.calcit
Length of output: 119
🏁 Script executed:
# Check the specific changes in the recent commit for compact.cirru
git show f035ac4 -- ./compact.cirru | head -100Repository: Respo/respo.calcit
Length of output: 4207
🏁 Script executed:
# Search for all usages of <> function in the codebase
rg '<>\s+' --type lisp -A 2 -B 2 | head -150Repository: Respo/respo.calcit
Length of output: 210
🏁 Script executed:
# Find all cirru files and search for <> usage
find . -name "*.cirru" -type f | head -10Repository: Respo/respo.calcit
Length of output: 90
🏁 Script executed:
# Search for <> function calls in cirru files
rg '<>' ./compact.cirru ./deps.cirru -B 1 -A 1 | head -200Repository: Respo/respo.calcit
Length of output: 5037
🏁 Script executed:
# Search for any .cr files that might be the source
find . -name "*.cr" -type f | head -20Repository: Respo/respo.calcit
Length of output: 44
🏁 Script executed:
# Check what files existed before this commit
git show f035ac4^:compact.cirru 2>&1 | head -20Repository: Respo/respo.calcit
Length of output: 100
🏁 Script executed:
# Check full git log
git log --oneline | head -20Repository: Respo/respo.calcit
Length of output: 119
Fix breaking change: <> now requires string content.
The assertion at line 770 will break existing call sites that pass keywords (e.g., <> |Remove, <> |Clear2) or variables not guaranteed to be strings (e.g., <> text, <> state). This includes the documented examples showing (<> text) and (<> value) which would now throw.
Consider coercing to string within the function rather than asserting, as shown in the suggested fix:
🛠️ Coerce to string inside `<>`
-defn <> (content ? style) (assert-type content :string)
- if (string? style)
- span $ {} (:inner-text content) (:class-name style)
- span $ {} (:inner-text content) (:style style)
+defn <> (content ? style)
+ let
+ text $ if (string? content) content (str content)
+ if (string? style)
+ span $ {} (:inner-text text) (:class-name style)
+ span $ {} (:inner-text text) (:style style)📝 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.
| defn <> (content ? style) (assert-type content :string) | |
| if (string? style) | |
| span $ {} (:inner-text content) (:class-name style) | |
| span $ {} (:inner-text content) (:style style) | |
| defn <> (content ? style) | |
| let | |
| text $ if (string? content) content (str content) | |
| if (string? style) | |
| span $ {} (:inner-text text) (:class-name style) | |
| span $ {} (:inner-text text) (:style style) |
🤖 Prompt for AI Agents
In `@compact.cirru` around lines 770 - 773, The defn for <> currently asserts
content is a string which breaks callers that pass keywords or non-string
values; remove the assert-type content :string and coerce content to a string
inside <> (e.g., call str on content once and use that variable for (:inner-text
content) placements) so existing usages like (<> text) or (<> |Remove) work;
keep the existing style handling logic (the branch that checks (string? style)
and chooses (:class-name style) vs (:style style)) unchanged.
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 `@deps.cirru`:
- Around line 2-5: The dependency entry for `@calcit/procs` is using a
non-existent version 0.11.5; update the version to a published one (e.g., 0.9.9)
in both deps.cirru (replace the |0.11.5 token for `@calcit/procs`) and
package.json (update the `@calcit/procs` version string in
dependencies/devDependencies), then run your package manager install to verify
resolution.
In `@llms/Calcit.md`:
- Line 916: Replace the incorrect subcommand string "cr query target-replace"
with the correct "cr tree target-replace" in the paragraph that instructs using
placeholders for long function edits; search for the exact phrase "cr query
target-replace" (the mistaken token) and update it to "cr tree target-replace"
so it matches the other references to target-replace (e.g., the earlier
occurrences that correctly use cr tree).
- Around line 769-772: The Markdown in llms/Calcit.md has a stray "**" inside
the bash fenced code block (the block containing "# Cirru one liner" and "cr
edit def app.core/multiply -e 'defn multiply (x y) (* x y)'") which merges the
next section header "# 基本操作:**" into the code fence; fix this by closing the
initial ```bash fence after the one-liner, remove the trailing "**" from the
header, render "基本操作:" as a bold or plain heading outside the code block, and
start a new ```bash fenced block for the following example commands (e.g., the
second "cr edit def 'app.core/multiply' -e 'defn multiply (x y) (* x y)'") so
examples and headings are properly separated.
---
Duplicate comments:
In `@llms/Calcit.md`:
- Line 200: The line flagged by MD036 (“**⚠️ 重要:生成 Cirru 代码前请先阅读语法指南**”) is
using emphasis markup as a heading; replace the emphasis with a proper Markdown
heading or plain text to resolve the rule (e.g. remove surrounding ** or convert
to a heading using a leading #), ensuring the text remains visible and
semantically a heading in the CalcIt.md content so the MD036 linter no longer
flags it.
- Around line 360-363: The fenced code block containing the bash example
starting with "```bash" and the command "cr tree replace ns/def -p '2' -e '(+ $
$)' --refer-original '$'" needs blank lines added immediately before the opening
```bash and immediately after the closing ``` to satisfy Markdown rule MD031;
insert one empty line above the "```bash" line and one empty line below the
trailing "```".
- Line 7: The line "**核心原则:用命令行工具(不要直接编辑文件),用 search 定位(比逐层导航快 10 倍)**" is using
strong emphasis as a heading (MD036); replace the emphasis with proper heading
syntax or plain text: remove the surrounding ** and add an appropriate Markdown
heading marker (e.g., "# " or "## ") before the text, or convert it into a
normal paragraph if it should not be a heading, so the linter stops flagging
MD036.
In `@package.json`:
- Line 6: The npm "test" script currently runs "cr --once ... js && node
test.mjs" but the ClojureScript compiler ("cr" invocation in the "test" script)
emits to js-out/ by default, so node will fail to find test.mjs; update the
"test" script in package.json to either (a) pass an explicit emit path to cr
(e.g. add --emit-path=. or similar flag to have cr write test.mjs to repo root)
or (b) point node at the actual output location produced by cr (e.g. node
js-out/test.mjs); modify the "test" script string accordingly so the two
commands reference the same output file.
Summary by CodeRabbit