feat: upd skills#437
Conversation
|
@skywardboundd is attempting to deploy a commit to the TOP Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR restructures documentation and development guidance by introducing two comprehensive skills: kit-dev for internal monorepo contributors (architecture, conventions, templates, testing) and ton-appkit for external library consumers (setup, API reference, recipes). The deprecated add-action-and-hook and add-ui-component skills are superseded, and contribution guides are updated to reference the new skills. ChangesKit-Dev Skill for Internal Monorepo Development
Ton-AppKit Skill for Library Consumer Integration
Contribution Guide Updates and Skill Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/appkit/skills/ton-appkit/evals.json (2)
34-41: 💤 Low valueConsider expanding jetton-send eval coverage.
Eval 3 currently has only 2 assertions, while similar mutation-based evals (e.g., eval 7 error-handling, eval 24 nft-transfer) have 4-5. Common jetton-send pitfalls that could be validated:
amountpassed as human-readable string (not raw units or bigint)isPendingused to disable the send buttonerror/isErrorhandling in the UIrecipientAddressformat validationThe existing assertions cover the core hook + cache pattern, so this is optional — the missing patterns are tested in other evals (18, 21, 28, 38).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/appkit/skills/ton-appkit/evals.json` around lines 34 - 41, The jetton-send eval ("id": 3, "name": "jetton-send") currently has only two assertions; add extra assertions to validate common mutation UI and input pitfalls: include an assertion checking that amount is passed as a human-readable string (e.g., "amount_is_human_readable"), one that verifies the send button is disabled when the mutation is pending (e.g., "uses_isPending_to_disable_send"), one for error handling in the UI (e.g., "shows_error_on_failure" or "handles_isError"), and one for recipient address validation (e.g., "validates_recipient_address_format"); update the "assertions" array in the eval definition to include these new assertion objects with brief "description" fields mirroring the examples above.
84-84: Missing eval ID 8 inpackages/appkit/skills/ton-appkit/evals.jsonEval IDs jump from
7to9(skipping8) around the reported line, and"id": 8doesn’t appear anywhere else underpackages/appkit/skills/. If IDs are intended to be contiguous, re-add/renumber the entry; otherwise document the intentional skip.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/appkit/skills/ton-appkit/evals.json` at line 84, The eval list in evals.json jumps from "id": 7 to "id": 9, so add or restore an entry with "id": 8 (or renumber subsequent entries to be contiguous) so IDs are sequential; locate the array entries around "id": 7 and "id": 9 in packages/appkit/skills/ton-appkit/evals.json and either insert the missing eval object with "id": 8 (populated like the nearby entries) or update "id" values from 9 onward to close the gap, and if the skip is intentional, add a comment/documentation in the file explaining the deliberate omission.packages/appkit/skills/ton-appkit/SKILL.md (1)
1-4: ⚡ Quick winConfirm
SKILL.mdis exempt from the kebab-case rule.
packages/appkit/**/*is supposed to use kebab-case filenames, and this entrypoint is the one exception here. If the loader doesn't require this exact casing, rename it before merge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/appkit/skills/ton-appkit/SKILL.md` around lines 1 - 4, This SKILL.md file is the intended exception to the packages/appkit/**/* kebab-case filename rule; verify the build/loader that enforces kebab-case accepts this exact filename (SKILL.md) for the ton-appkit entrypoint, and if it does not, either update the loader/config to whitelist SKILL.md as an exception or rename the file to a kebab-case alternative and update any imports/references to that symbol; ensure the chosen fix is applied consistently so the bundler/loader no longer fails on the ton-appkit entrypoint.Source: Coding guidelines
🤖 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 @.claude/skills/kit-dev/skill-reference/recipes.md:
- Around line 28-71: The watch template must accept and forward resource
options: change watchXxx signature to accept parameters that extend
GetXxxOptions (e.g., { ...GetXxxOptions, onChange, onError? }), pass those
options into getXxx(appKit, options) when refreshing, and mark
cancelled/unsubscribe as before; also update Pattern B hook invalidation to
compute/invalidate using getXxxQueryKey(options) (or call handleXxxUpdate with
the same options) so the query key remains scoped to the resource inputs instead
of being global. Ensure you reference the existing types GetXxxReturnType and
GetXxxOptions (or create them if missing) and update any unsubscribe logic
(unsubscribeA/unsubscribeB) unchanged.
In `@packages/appkit/skills/ton-appkit/SKILL.md`:
- Line 34: Update the Next.js hydration guidance in SKILL.md by removing the
unsupported "ssr: true" hint and leaving the mount-gate / dynamic-import pattern
only; specifically, edit the table row that currently reads "'use client'
providers + `ssr: true` + mount gate or dynamic import" to drop the `ssr: true`
fragment and ensure any mention of AppKitConfig does not imply an `ssr` option,
keeping the advice focused on using mount gates or dynamic imports for
client-only providers.
---
Nitpick comments:
In `@packages/appkit/skills/ton-appkit/evals.json`:
- Around line 34-41: The jetton-send eval ("id": 3, "name": "jetton-send")
currently has only two assertions; add extra assertions to validate common
mutation UI and input pitfalls: include an assertion checking that amount is
passed as a human-readable string (e.g., "amount_is_human_readable"), one that
verifies the send button is disabled when the mutation is pending (e.g.,
"uses_isPending_to_disable_send"), one for error handling in the UI (e.g.,
"shows_error_on_failure" or "handles_isError"), and one for recipient address
validation (e.g., "validates_recipient_address_format"); update the "assertions"
array in the eval definition to include these new assertion objects with brief
"description" fields mirroring the examples above.
- Line 84: The eval list in evals.json jumps from "id": 7 to "id": 9, so add or
restore an entry with "id": 8 (or renumber subsequent entries to be contiguous)
so IDs are sequential; locate the array entries around "id": 7 and "id": 9 in
packages/appkit/skills/ton-appkit/evals.json and either insert the missing eval
object with "id": 8 (populated like the nearby entries) or update "id" values
from 9 onward to close the gap, and if the skip is intentional, add a
comment/documentation in the file explaining the deliberate omission.
In `@packages/appkit/skills/ton-appkit/SKILL.md`:
- Around line 1-4: This SKILL.md file is the intended exception to the
packages/appkit/**/* kebab-case filename rule; verify the build/loader that
enforces kebab-case accepts this exact filename (SKILL.md) for the ton-appkit
entrypoint, and if it does not, either update the loader/config to whitelist
SKILL.md as an exception or rename the file to a kebab-case alternative and
update any imports/references to that symbol; ensure the chosen fix is applied
consistently so the bundler/loader no longer fails on the ton-appkit entrypoint.
🪄 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: ae518727-cd5b-41ba-b1fd-3ccc485b845c
📒 Files selected for processing (11)
.claude/skills/add-action-and-hook/SKILL.md.claude/skills/add-ui-component/SKILL.md.claude/skills/kit-dev/SKILL.md.claude/skills/kit-dev/evals.json.claude/skills/kit-dev/skill-reference/recipes.mdCLAUDE.mdpackages/appkit-react/CLAUDE.mdpackages/appkit/CLAUDE.mdpackages/appkit/skills/ton-appkit/SKILL.mdpackages/appkit/skills/ton-appkit/evals.jsonpackages/appkit/skills/ton-appkit/skill-reference/recipes.md
💤 Files with no reviewable changes (2)
- .claude/skills/add-action-and-hook/SKILL.md
- .claude/skills/add-ui-component/SKILL.md
|
waiting for #454 |
Summary by CodeRabbit