Conversation
📝 WalkthroughWalkthroughThis change adds backtick and operator tokenization, expands parser support for modifiers and attributes, tightens TypeScript types, adds linting and coverage configuration, updates package tooling, and introduces CI, deployment, issue templates, and repository guidance. ChangesParser and language support
Development tooling
Repository workflows and guidance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Source
participant Tokenizer
participant Parser
participant AST
Source->>Tokenizer: Read strings and operators
Tokenizer->>Parser: Emit typed tokens
Parser->>AST: Build modifiers and nodes
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Build/src/parser/parser.ts (1)
281-285: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle
BACKTICK_STRINGinparseExpression()
Build/src/parser/tokenizer.tsemits backtick-quoted literals, andparseStateDeclaration()/parseDerivedDeclaration()pass them throughparseExpression(). Right now they lose their delimiters and serialize as bare identifiers; mirror theparseBlockBody()handling here.🤖 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 `@Build/src/parser/parser.ts` around lines 281 - 285, parseExpression() currently JSON-serializes only STRING tokens, so BACKTICK_STRING values lose their delimiters. Update the token handling branch in parseExpression() to treat BACKTICK_STRING like STRING, mirroring parseBlockBody(), while preserving the existing behavior for other token types.
🧹 Nitpick comments (4)
Build/src/parser/parser.ts (1)
434-498: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winModifier parsing logic is duplicated with
parseInlineModifierinmodifiers.ts.The
@style,@if,@class,@each, and@bindbranches inparseModifiers(lines 420-498) mirror the same logic inparseInlineModifier(Build/src/parser/atcodes/modifiers.ts:75-145). Any future change to one must be replicated in the other, risking drift.Consider extracting shared parsing helpers (e.g.,
parseStyleModifier,parseIfModifier,parseEachModifier) that return the modifier object, with each caller deciding whether to push or return.🤖 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 `@Build/src/parser/parser.ts` around lines 434 - 498, Deduplicate modifier parsing between parseModifiers and parseInlineModifier by extracting shared helpers for `@style`, `@if`, `@class`, `@each`, and `@bind` (such as parseStyleModifier, parseIfModifier, and parseEachModifier) in the parser modifier utilities. Have each caller invoke these helpers and then push or return the resulting modifier as appropriate, preserving existing token validation and error messages.Build/src/parser/tokenizer.ts (1)
110-133: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist
SYMBOLSoutside thewhileloop.The
SYMBOLSobject is recreated on every iteration of the main tokenization loop, meaning O(n) allocations for an input of length n. Moving it to module scope (or just before the loop) eliminates this overhead entirely.♻️ Proposed refactor
+const SYMBOLS: Record<string, TokenType> = { + "<": "LT", + ">": "GT", + "{": "LBRACE", + "}": "RBRACE", + "(": "LPAREN", + ")": "RPAREN", + "[": "LBRACKET", + "]": "RBRACKET", + ":": "COLON", + ";": "SEMI", + ",": "COMMA", + "@": "AT", + "=": "EQUALS", + ".": "DOT", + "+": "PLUS", + "-": "MINUS", + "*": "STAR", + "|": "PIPE", + "&": "AMPERSAND", + "!": "BANG", + "?": "QUESTION", + "%": "PERCENT", +}; + export function tokenize(input: string): Token[] { const tokens: Token[] = []; let i = 0; let line = 1; let col = 1; while (i < input.length) { const ch = input[i]; @@ - const SYMBOLS: Record<string, TokenType> = { - "<": "LT", - ">": "GT", - "{": "LBRACE", - "}": "RBRACE", - "(": "LPAREN", - ")": "RPAREN", - "[": "LBRACKET", - "]": "RBRACKET", - ":": "COLON", - ";": "SEMI", - ",": "COMMA", - "@": "AT", - "=": "EQUALS", - ".": "DOT", - "+": "PLUS", - "-": "MINUS", - "*": "STAR", - "|": "PIPE", - "&": "AMPERSAND", - "!": "BANG", - "?": "QUESTION", - "%": "PERCENT", - }; - if (SYMBOLS[ch]) {🤖 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 `@Build/src/parser/tokenizer.ts` around lines 110 - 133, Move the SYMBOLS constant out of the main tokenization while loop to module scope or immediately before the loop, preserving its existing mappings and references so it is allocated only once.Build/src/errors.ts (1)
7-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider making
getLoggerreturn a non-null type.
getLoggeralways assignsloggerbefore returning, but its inferred return type is nullable, forcing!at every call site. Narrowing the return type eliminates the assertions and makes null-safety explicit.♻️ Proposed refactor
-function getLogger(scope: string) { +function getLogger(scope: string): NonNullable<typeof logger> { if (!logger) { try { // eslint-disable-next-line `@typescript-eslint/no-require-imports` const satori = require("`@nisoku/satori`"); const s = satori.createSatori({ logLevel: "error", enableConsole: true }); logger = s.createLogger(scope); } catch { logger = { info: (msg: string, opts?: unknown) => console.log(`[${scope}] ${msg}`, opts), warn: (msg: string, opts?: unknown) => console.warn(`[${scope}] ${msg}`, opts), error: (msg: string, opts?: unknown) => console.error(`[${scope}] ${msg}`, opts), }; } } return logger; }Then remove the
!at lines 46, 62, 75, and 86:- const log = getLogger("sakko")!; + const log = getLogger("sakko");🤖 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 `@Build/src/errors.ts` around lines 7 - 26, Update getLogger to explicitly return the non-null logger type, reflecting that both initialization paths assign logger before returning. Then remove the non-null assertions at the affected getLogger call sites.Build/src/runtime/register.ts (1)
42-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
_modulePathparameter
createFactoryFromCodenever reads_modulePath, and the value passed at the call site is unused too. Drop it from the signature and the call to avoid dead API surface.🤖 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 `@Build/src/runtime/register.ts` around lines 42 - 45, Remove the unused _modulePath parameter from createFactoryFromCode and update every call site to stop passing it, preserving the remaining arguments and 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 @.github/workflows/ci.yml:
- Around line 35-45: Add a CI step invoking `npm run typecheck` in the `Build`
working directory, alongside the existing `Lint`, `Run tests`, and `Build`
steps, so TypeScript errors are validated before merging.
- Around line 21-22: Add persist-credentials: false to the actions/checkout@v4
step named Checkout in the CI workflow, ensuring the token is not stored in the
repository’s Git configuration.
In `@Build/src/parser/parser.ts`:
- Around line 217-224: Update the separator condition in parseBlockBody to treat
BACKTICK_STRING the same as STRING when checking prevToken.type, while retaining
the existing line, token type, and separator logic.
In `@CODE_OF_CONDUCT.md`:
- Around line 14-16: Add a concrete, private reporting channel to the
Enforcement section of CODE_OF_CONDUCT.md, replacing the vague “contacting the
project team” wording with a project-specific email address, contact link, or
explicit instructions for reporting abusive behavior.
In `@CONTRIBUTING.md`:
- Around line 7-10: Replace the placeholder clone URL in the contribution steps
with the actual repository URL https://github.com/Nisoku/Sakko.git, while
keeping the surrounding setup instructions unchanged.
- Around line 24-27: Update the CI workflow to run the existing typecheck script
alongside lint, tests, and build, keeping it consistent with the requirements
documented in CONTRIBUTING.md. Use the package’s npm typecheck command in the
relevant workflow job, or alternatively revise the guide if CI intentionally
will not enforce typechecking.
---
Outside diff comments:
In `@Build/src/parser/parser.ts`:
- Around line 281-285: parseExpression() currently JSON-serializes only STRING
tokens, so BACKTICK_STRING values lose their delimiters. Update the token
handling branch in parseExpression() to treat BACKTICK_STRING like STRING,
mirroring parseBlockBody(), while preserving the existing behavior for other
token types.
---
Nitpick comments:
In `@Build/src/errors.ts`:
- Around line 7-26: Update getLogger to explicitly return the non-null logger
type, reflecting that both initialization paths assign logger before returning.
Then remove the non-null assertions at the affected getLogger call sites.
In `@Build/src/parser/parser.ts`:
- Around line 434-498: Deduplicate modifier parsing between parseModifiers and
parseInlineModifier by extracting shared helpers for `@style`, `@if`, `@class`, `@each`,
and `@bind` (such as parseStyleModifier, parseIfModifier, and parseEachModifier)
in the parser modifier utilities. Have each caller invoke these helpers and then
push or return the resulting modifier as appropriate, preserving existing token
validation and error messages.
In `@Build/src/parser/tokenizer.ts`:
- Around line 110-133: Move the SYMBOLS constant out of the main tokenization
while loop to module scope or immediately before the loop, preserving its
existing mappings and references so it is allocated only once.
In `@Build/src/runtime/register.ts`:
- Around line 42-45: Remove the unused _modulePath parameter from
createFactoryFromCode and update every call site to stop passing it, preserving
the remaining arguments and behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce0c0015-c004-41c8-b92a-d1697edd21ab
⛔ Files ignored due to path filters (1)
Build/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
.github/ISSUE_TEMPLATE/bug_report.yml.github/ISSUE_TEMPLATE/feature_request.yml.github/PULL_REQUEST_TEMPLATE.md.github/workflows/ci.yml.github/workflows/pages.yml.nvmrcBuild/README.mdBuild/eslint.config.tsBuild/jest.config.jsBuild/package.jsonBuild/src/compiler/atcode.tsBuild/src/compiler/component.tsBuild/src/errors.tsBuild/src/index.tsBuild/src/parser/atcodes/modifiers.tsBuild/src/parser/elements/container.tsBuild/src/parser/elements/inline.tsBuild/src/parser/parser.tsBuild/src/parser/tokenizer.tsBuild/src/parser/types.tsBuild/src/runtime/register.tsBuild/tests/atcode.parser.test.tsBuild/tests/atcode.tokenizer.test.tsBuild/tests/errors.test.tsBuild/tests/parser.test.tsBuild/vite.config.tsCHANGELOG.mdCODE_OF_CONDUCT.mdCONTRIBUTING.mdSECURITY.md
💤 Files with no reviewable changes (1)
- Build/src/compiler/atcode.ts
| - name: Checkout | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable credential persistence for checkout.
The workflow executes dependency installation and build steps on checked-out code. Persisting GITHUB_TOKEN in .git/config allows malicious lifecycle scripts or build tooling to access it. Add persist-credentials: false.
Proposed fix
- name: Checkout
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 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.
| - name: Checkout | |
| uses: actions/checkout@v4 | |
| - name: Checkout | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 21-22: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/ci.yml around lines 21 - 22, Add persist-credentials:
false to the actions/checkout@v4 step named Checkout in the CI workflow,
ensuring the token is not stored in the repository’s Git configuration.
Source: Linters/SAST tools
| - name: Lint | ||
| working-directory: Build | ||
| run: npm run lint | ||
|
|
||
| - name: Run tests | ||
| working-directory: Build | ||
| run: npm test | ||
|
|
||
| - name: Build | ||
| working-directory: Build | ||
| run: npm run build |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run the TypeScript check in CI.
Build/package.json defines npm run typecheck, but this workflow only runs lint, tests, and Vite build. Type errors can therefore merge successfully despite the PR tightening TypeScript types.
Proposed fix
- name: Lint
working-directory: Build
run: npm run lint
+ - name: Typecheck
+ working-directory: Build
+ run: npm run typecheck
+
- name: Run tests📝 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.
| - name: Lint | |
| working-directory: Build | |
| run: npm run lint | |
| - name: Run tests | |
| working-directory: Build | |
| run: npm test | |
| - name: Build | |
| working-directory: Build | |
| run: npm run build | |
| - name: Lint | |
| working-directory: Build | |
| run: npm run lint | |
| - name: Typecheck | |
| working-directory: Build | |
| run: npm run typecheck | |
| - name: Run tests | |
| working-directory: Build | |
| run: npm test | |
| - name: Build | |
| working-directory: Build | |
| run: npm run build |
🤖 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 @.github/workflows/ci.yml around lines 35 - 45, Add a CI step invoking `npm
run typecheck` in the `Build` working directory, alongside the existing `Lint`,
`Run tests`, and `Build` steps, so TypeScript errors are validated before
merging.
| if ( | ||
| prevToken && | ||
| prevToken.line < token.line && | ||
| prevToken.type === "STRING" && | ||
| token.type === "IDENT" | ||
| ) { | ||
| body += ";\n"; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
parseBlockBody separator misses BACKTICK_STRING tokens.
The ;\n insertion condition checks prevToken.type === "STRING" but not "BACKTICK_STRING". When a backtick string ends a line and the next line starts with an IDENT, no separator is inserted, producing concatenated invalid JS. For example:
`@effect` {
document.title = `Count: ${count}`
console.log("done")
}
Serializes to document.title = `Count: ${count}`console.log("done") instead of document.title = `Count: ${count}`;\nconsole.log("done").
🐛 Proposed fix
if (
prevToken &&
prevToken.line < token.line &&
- prevToken.type === "STRING" &&
+ (prevToken.type === "STRING" || prevToken.type === "BACKTICK_STRING") &&
token.type === "IDENT"
) {
body += ";\n";
}📝 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.
| if ( | |
| prevToken && | |
| prevToken.line < token.line && | |
| prevToken.type === "STRING" && | |
| token.type === "IDENT" | |
| ) { | |
| body += ";\n"; | |
| } | |
| if ( | |
| prevToken && | |
| prevToken.line < token.line && | |
| (prevToken.type === "STRING" || prevToken.type === "BACKTICK_STRING") && | |
| token.type === "IDENT" | |
| ) { | |
| body += ";\n"; | |
| } |
🤖 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 `@Build/src/parser/parser.ts` around lines 217 - 224, Update the separator
condition in parseBlockBody to treat BACKTICK_STRING the same as STRING when
checking prevToken.type, while retaining the existing line, token type, and
separator logic.
| ## Enforcement | ||
|
|
||
| Project maintainers are responsible for clarifying and enforcing standards. Instances of abusive behavior may be reported by contacting the project team. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Provide a concrete reporting channel.
“Contacting the project team” does not tell contributors where or how to report abuse. Add a private email address, contact link, or explicit reporting instructions.
🤖 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 `@CODE_OF_CONDUCT.md` around lines 14 - 16, Add a concrete, private reporting
channel to the Enforcement section of CODE_OF_CONDUCT.md, replacing the vague
“contacting the project team” wording with a project-specific email address,
contact link, or explicit instructions for reporting abusive behavior.
| 1. Fork the repository | ||
| 2. Clone your fork: `git clone https://github.com/your-username/Sakko.git` | ||
| 3. Install dependencies: `cd Build && npm install` | ||
| 4. Create a feature branch: `git checkout -b feat/your-feature` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the repository clone URL instead of a placeholder.
Copied literally, this command targets github.com/your-username/Sakko.git rather than the repository referenced by this PR (github.com/Nisoku/Sakko.git).
Proposed fix
-2. Clone your fork: `git clone https://github.com/your-username/Sakko.git`
+2. Clone your fork: `git clone https://github.com/Nisoku/Sakko.git`📝 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. Fork the repository | |
| 2. Clone your fork: `git clone https://github.com/your-username/Sakko.git` | |
| 3. Install dependencies: `cd Build && npm install` | |
| 4. Create a feature branch: `git checkout -b feat/your-feature` | |
| 1. Fork the repository | |
| 2. Clone your fork: `git clone https://github.com/Nisoku/Sakko.git` | |
| 3. Install dependencies: `cd Build && npm install` | |
| 4. Create a feature branch: `git checkout -b feat/your-feature` |
🤖 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 `@CONTRIBUTING.md` around lines 7 - 10, Replace the placeholder clone URL in
the contribution steps with the actual repository URL
https://github.com/Nisoku/Sakko.git, while keeping the surrounding setup
instructions unchanged.
| - Keep changes focused. One feature or fix per PR. | ||
| - Add tests for new functionality | ||
| - Ensure all checks pass (test, lint, typecheck, build) | ||
| - Update CHANGELOG.md if applicable |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files | rg '^(CONTRIBUTING\.md|.*\.github/workflows/.*\.ya?ml)$'
printf '\n== CONTRIBUTING.md around the cited lines ==\n'
nl -ba CONTRIBUTING.md | sed -n '1,80p'
printf '\n== Workflow files mentioning lint/test/build/typecheck ==\n'
rg -n --glob '.github/workflows/*.{yml,yaml}' 'typecheck|lint|test|build|npm run' .github/workflowsRepository: Nisoku/Sakko
Length of output: 348
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== CONTRIBUTING.md around the cited lines ==\n'
sed -n '20,35p' CONTRIBUTING.md | cat -n
printf '\n== .github/workflows/ci.yml ==\n'
sed -n '1,220p' .github/workflows/ci.yml | cat -n
printf '\n== Search for typecheck in workflows ==\n'
rg -n 'typecheck|lint|test|build|npm run' .github/workflows/ci.yml .github/workflows/pages.yml .github/workflows/release.ymlRepository: Nisoku/Sakko
Length of output: 2634
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Build/package.json scripts ==\n'
sed -n '1,220p' Build/package.json | cat -n
printf '\n== Search for typecheck script references ==\n'
rg -n '"typecheck"|typecheck' Build/package.json Build .github/workflowsRepository: Nisoku/Sakko
Length of output: 2117
Add npm run typecheck to CI. CONTRIBUTING.md requires tests, lint, typecheck, and build, but .github/workflows/ci.yml only runs lint, tests, and build. Build/package.json already defines a typecheck script, so wire it into the workflow or relax the guide.
🤖 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 `@CONTRIBUTING.md` around lines 24 - 27, Update the CI workflow to run the
existing typecheck script alongside lint, tests, and build, keeping it
consistent with the requirements documented in CONTRIBUTING.md. Use the
package’s npm typecheck command in the relevant workflow job, or alternatively
revise the guide if CI intentionally will not enforce typechecking.
1 participant
Summary by CodeRabbit
New Features
@style,@if,@class, and@eachmodifiers.data-*attributes.!,?,%,|, and&.Documentation
Quality Improvements