Skip to content

chore(upstream): sync caffeinelabs/skills 9274f9b → 38a1136 - #372

Merged
raymondk merged 1 commit into
mainfrom
chore/sync-upstream-skills-38a1136
Sep 1, 2026
Merged

chore(upstream): sync caffeinelabs/skills 9274f9b → 38a1136#372
raymondk merged 1 commit into
mainfrom
chore/sync-upstream-skills-38a1136

Conversation

@marc0olo

@marc0olo marc0olo commented Aug 31, 2026

Copy link
Copy Markdown
Member

Syncs caffeinelabs/skills from 9274f9b38a1136.

Closes #371

Scope

Skill Upstream change Action
writing-motoko 0.1.80.2.0, 4 files changed Content synced
reviewing-motoko none — byte-identical to our 90a2433 pin Pin bumped, pin note dropped
migrating-motoko-actors none Pin bumped
troubleshooting-motoko-migrations none Pin bumped

All four now share one pinned commit again, so the reviewing-motoko pin note about temporarily diverging pins is removed.

writing-motoko content changes

Owned sections

All re-applied, none now covered upstream: frontmatter (our description / license / string compatibility / metadata), references/ link paths, docs.mops.one → "Load the mops-cli skill", ## Additional Resources## Additional References plus the mops tooling bullet.

A note on "42".toNat()

references/type-conversions.md now spells Text parsing as a receiver call:

let maybeNat = "42".toNat(); // ?Nat
let maybeInt = "-5".toInt(); // ?Int

This is correct. toNat/toInt with a Text receiver are defined in the target modules — Nat.mo:68 is public let toNat : (self : Text) -> ?Nat = fromText; ("meant to be used with contextual-dot notation"), and Int.mo:118 is the equivalent for toInt. Our own synced api-reference.md lists them at line 385 (## Nat) and line 184 (## Int), since it groups by defining module rather than by receiver type.

An earlier revision of this description claimed the snippet was broken. That was my error — I searched Text.mo and the ## Text section, found nothing, and read absence as proof. Retracted upstream in caffeinelabs/skills#10.

One genuine caveat survives, and it is a documentation gap rather than a defect: the module that must be imported is the one that defines the function, not the receiver's. "42".toNat() needs import Nat "mo:core/Nat", and bytes.toBlob() needs Array (toBlob : (self : [Nat8]) -> Blob lives there). --implicit-package=core removes the requirement entirely — moc describes it as "allow contextual dot and implicits resolution from all modules in the given package" — and the Caffeine template sets it, but references/project-setup.md does not mention it. Raised upstream on #10; tracked on our side for a follow-up.

Evaluations

Three new cases seeded from the diff — the array contains reversal, the toX conversion rule, and the new Result guidance are exactly where an agent hallucinates without the updated skill.

New cases — with skill vs baseline
Case With skill Baseline
25. Arrays have contains for membership 4/4 1/4
26. Numeric conversion uses toX on the source value 4/4 1/4
27. Result error type is a variant, not Text 5/5 3/5
━━━ Arrays have contains for membership ━━━
  WITH skill: 4/4 passed
    ✅ Uses `tags.contains("urgent")` — `[T]` arrays DO have `contains` in mo:core
    ✅ Passes only the element — `equal` is an implicit argument
    ✅ Does NOT claim arrays have no `contains`
    ✅ Does NOT fall back to `.indexOf(...) != null` / `.any(...)` as the primary answer
  WITHOUT skill: 1/4 passed
    ❌ The primary answer uses `Array.find(tags, ...) != null` and never uses `contains`.
    ❌ The only mention of `contains` explicitly passes `Text.equal` as a third argument.
    ❌ The primary answer is a manual `Array.find(...) != null` search.

━━━ Numeric conversion uses toX on the source value ━━━
  WITH skill: 4/4 passed
    ✅ Uses receiver conversions: sum.toFloat() / count.toFloat()
    ✅ Does NOT use deprecated Module.fromX calls
    ✅ Does NOT chain through sized numeric modules (Nat64/Int64)
    ✅ Imports mo:core/Nat, not Float/Int64/Nat64
  WITHOUT skill: 1/4 passed
    ❌ Uses Float.fromInt(sum) / Float.fromInt(count) instead of toFloat().
    ❌ Explicitly uses the deprecated Float.fromInt(...).
    ❌ Imports mo:core/Float, not mo:core/Nat.

━━━ Result error type is a variant, not Text ━━━
  WITH skill: 5/5 passed
    ✅ Returns `async Result.Result<Ok, Err>` using `mo:core/Result`
    ✅ Declares the error as a variant with one tag per failure
    ✅ Does NOT use `Result<Ok, Text>` as the discriminator
    ✅ Does NOT propose trapping for caller-fixable failures
    ✅ Uses `public shared ({ caller }) func`
  WITHOUT skill: 3/5 passed
    ❌ No import from mo:core/Result is shown, so the source of Result is unconfirmed.
    ❌ Uses `public shared (msg) func`, which does not destructure the caller.
Regression check — existing cases covering rewritten content

The contains guidance and the equality section were both rewritten, so the two existing cases covering them were re-run:

━━━ contains vs find ━━━
  WITH skill: 2/2 passed

━━━ Prefer equal/compare over == on records with var fields ━━━
  WITH skill: 4/4 passed

npm run validate: 30 skills, all passed (21 warnings, unchanged from baseline).

writing-motoko 0.1.8 → 0.2.0. reviewing-motoko, migrating-motoko-actors
and troubleshooting-motoko-migrations have no content changes — their pins
are bumped so all four share one commit again.

Body changes (upstream, applied verbatim):
- Remove the Caffeine-specific "Build feedback" section (upstream #8)
- Arrays DO have .contains(element) — reverses prior guidance in two places
  and the M0096 error-table row
- Numeric conversions are toX on the source value, with a from→to mapping
  table; `field fromInt is deprecated` row generalizes to `field fromX`
- New "Error Handling: Result" section (variant Err, no trap on caller
  error, chain via mapOk/chain/fromOption)
- Equality reframed as "equal/compare vs ==", permitting == on primitives
  and shared fields (upstream #9.2)
- Restore `let ages = Map.empty<Text, Nat>()` and add `type Point` to the
  implicit-parameter examples (upstream #9.1, #9.3)
- api-reference.md regenerated from motoko-core's API lock: new Blob, Bool,
  Char, Error, Float, Result, Timer sections; deprecated fromX entries
  dropped; Principal.fromActor signature complete (upstream #5.1)

Owned sections re-applied: frontmatter (compatibility → core >= 2.6.0),
references/ link paths, docs.mops.one → mops-cli skill, Additional
References rename + mops tooling bullet.

Evals: three new cases seeded from the diff (array contains, toX
conversions, Result error variants), each 100% with skill vs 1/4, 1/4 and
3/5 baseline.
@github-actions

Copy link
Copy Markdown

Skill Validation Report

Validating skill: /home/runner/work/icskills/icskills/skills/writing-motoko

Structure

  • Pass: SKILL.md found
  • Pass: internal link: references/reserved-keywords.md (exists)
  • Pass: internal link: references/api-reference.md (exists)
  • Pass: internal link: references/project-setup.md (exists)
  • Pass: internal link: references/equality.md (exists)
  • Pass: all files in references/ are referenced

Frontmatter

  • Pass: name: "writing-motoko" (valid)
  • Pass: description: (448 chars)
  • Pass: license: "Apache-2.0"
  • Pass: compatibility: (43 chars)
  • Pass: metadata: (2 entries)

Tokens

  • Warning: SKILL.md body is 11674 tokens (spec recommends < 5000)
  • Warning: SKILL.md body is 864 lines (spec recommends < 500)
  • Warning: references/api-reference.md is 12038 tokens — consider splitting into smaller focused files so agents load only what they need

Markdown

  • Pass: no unclosed code fences found

Tokens

File Tokens
SKILL.md body 11,674
references/api-reference.md 12,038
references/control-flow.md 927
references/equality.md 1,212
references/examples.md 4,523
references/project-setup.md 530
references/reserved-keywords.md 440
references/type-conversions.md 495
Total 31,839

Content Analysis

Metric Value
Word count 7,007
Code block ratio 0.38
Imperative ratio 0.13
Information density 0.25
Instruction specificity 0.87
Sections 41
List items 63
Code blocks 43

References Content Analysis

Metric Value
Word count 11,364
Code block ratio 0.10
Imperative ratio 0.07
Information density 0.09
Instruction specificity 0.61
Sections 59
List items 564
Code blocks 40

Contamination Analysis

Metric Value
Contamination level low
Contamination score 0.00
Scope breadth 1

References Contamination Analysis

Metric Value
Contamination level medium
Contamination score 0.30
Primary language category config
Scope breadth 2
  • Multi-interface tool detected: stripe

Result: 3 warnings

Project Checks


✓ Project checks passed for 1 skills (0 warnings)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Syncs the writing-motoko skill (and related upstream pin metadata) to match caffeinelabs/skills at commit 38a1136, updating Motoko guidance and regenerating the mo:core API reference so the skill stays aligned with core >= 2.6.0.

Changes:

  • Updated writing-motoko guidance (arrays .contains, equality guidance, Result-based error handling, numeric conversion rules) and removed the Caffeine-specific build feedback section.
  • Regenerated/expanded the mo:core API reference and updated compatibility to core >= 2.6.0.
  • Added new evaluation cases targeting the updated guidance and updated the upstream pin tracking metadata.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
skills/writing-motoko/SKILL.md Updates core guidance (contains/equality/Result/numeric conversions) and removes build-feedback section.
skills/writing-motoko/references/type-conversions.md Updates conversion guidance and examples to emphasize receiver-style toX conversions.
skills/writing-motoko/references/equality.md Reframes guidance on == vs equal/compare and adds an example type.
skills/writing-motoko/references/api-reference.md Regenerates mo:core API signatures, omitting deprecated APIs and adding new sections.
evaluations/writing-motoko.json Adds eval cases for arrays contains, toX conversions, and Result error typing.
.claude/upstream.md Bumps upstream pinned commit/version and updates last-synced metadata/notes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread skills/writing-motoko/SKILL.md
Comment thread skills/writing-motoko/SKILL.md
Comment thread skills/writing-motoko/references/type-conversions.md
Comment thread skills/writing-motoko/references/type-conversions.md
@marc0olo

Copy link
Copy Markdown
Member Author

Checked all four Copilot comments against motoko-core and the skill's own rules. Two are already filed upstream, one is a new genuine defect now filed, and one is incorrect — its suggested fix would introduce a defect the skills classify as a Blocker.

None of these are patched in this PR: per the sync policy, body content is not icskills-owned, so genuine defects in synced files are filed upstream rather than patched locally.


1. SKILL.md:626Result described as unconditionally shared — ✅ correct, filed as caffeinelabs/skills#11

Result carries no sharedness constraint: Result.mo:46 is public type Result<Ok, Err> = Types.Result<Ok, Err> and Types.mo:41 is public type Result<T, E> = { #ok : T; #err : E }. A variant is shared only if its payloads are.

This matters more than it might look, because the skill teaches at length that internal records carry var fields and cannot cross a public signature. So a reader following "it is a shared type … no wrapper needed" writes Result<Task, BookingError> with a var field in Task and hits a compile error the doc just told them wouldn't happen. The section's own second example already gets this right — it returns Result<RoomView, …> and defines toView to narrow it — so only the prose overstates it.

2. SKILL.md:666 — "let rooms : Map.Map<Nat, Room>; needs an initializer" — ❌ incorrect, not filed

This is correct Motoko as written. Under enhanced migration, stable actor fields are declared type-only and every initial value comes from the migration chain. This skill states it in five places — SKILL.md:23 ("Inline initializers on stable actor fields" is listed under NEVER), :43, :721 ("declared with types only — no initializers"), :746, and :858 (Critical Requirement 10) — and reviewing-motoko S1 classifies an inline initializer on a stable field as a Blocker, failing with M0250 / M0014.

Adding Map.empty() as suggested would introduce exactly that Blocker. Declining this one.

3. type-conversions.md:53"42".toNat() — ✅ right conclusion, already filed as #10

Already documented in this PR's description as a known defect in the synced content.

One correction to the diagnosis, though: the comment says the examples "rely on Nat/Int being in scope (the functions live on those modules)", implying an import would fix it. It wouldn't. Contextual dot notation resolves on the receiver's type, so "42".toNat() looks for toNat in Text — and Text has no toNat/toInt at all (verified in motoko-core/src/Text.mo and in this commit's own regenerated api-reference.md, whose ## Text section lists 41 entries and neither of these). No import makes it compile; the correct call is the Nat.fromText(...) the rewrite removed, which is still current and not deprecated.

4. type-conversions.md:5 — narrow the Module.fromX deprecation claim — ✅ correct, already filed as #10

This matches the "Scope note on the surrounding rule" section of #10 almost exactly, including the suggested remedy of scoping the rule to the sized-numeric conversions. Nice independent confirmation.


One more, found while verifying

Not raised by the review: the same result-chain example declares public query func book(...), while the first snippet in that section declares the same function as public shared ({ caller }) func book(...). Beyond the inconsistency, a book that calls reserve is a state mutation declared as a query — harmless in the snippet since reserve is a stub, but a reader filling in a real reserve gets a reservation that is silently discarded when the query completes. Filed as the second point of #11.

Upstream issue status after this PR

Issue Status
#8 Caffeine "Build feedback" Closed — fixed at 38a1136
#9 equality + ages regression Closed — all 3 points fixed
#5 copy-paste correctness 2 of 3 fixed; asked upstream whether the last is intentional
#3 dropped Motoko items M0141 outstanding; asked upstream whether intentional
#10 "42".toNat() regression Filed
#11 Result sharedness + query mutation Filed

@marc0olo

marc0olo commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

Correction to my review reply above

A caffeinelabs/skills maintainer pointed out that "42".toNat() compiles — you just need Nat in scope (or --implicit-package=core). They're right, my analysis above was wrong, and Copilot's comment #3 was correct.

toNat/toInt with a Text receiver are defined in the target modules, not in Text:

// motoko-core/src/Nat.mo:68
/// This functions is meant to be used with contextual-dot notation.
/// assert "1234".toNat() == ?1234;
public let toNat : (self : Text) -> ?Nat = fromText;

// motoko-core/src/Int.mo:118
public func toInt(self : Text) : ?Int { fromText(self) };

I assumed a receiver-typed function lives in the receiver's module, searched Text.mo and the ## Text section of api-reference.md, and read absence as proof. But the reference groups by defining module — toNat is at line 385 under ## Nat and toInt at line 184 under ## Int, in the same file I cited as evidence against them. My three "independent confirmations" were one error counted three times.

Corrections to the four review comments, restated accurately:

# Comment Correct verdict
1 Result described as unconditionally shared ✅ Valid — #11 stands, unaffected by this
2 let rooms : Map.Map<Nat, Room>; needs an initializer ❌ Still incorrect — type-only declaration is required under EM
3 "42".toNat() relies on Nat/Int being in scope Correct, and I was wrong to dismiss the mechanism. This is exactly right: the functions do live on those modules, and without an import (or --implicit-package=core) the snippet does not compile
4 Narrow the Module.fromX deprecation claim ⚠️ Weaker than I said — TextNat/Int does have a receiver form, which is why the rule was written that way. Some genuine fromX constructors remain (Text.fromArray, Principal.fromText), so it's mildly over-general, but not the defect I described

So: only comment 2 is actually wrong, and my reply above got comments 3 and 4 backwards. #10 is retracted and closed; the PR description is updated.

Nothing in the synced content needs changing — this branch is correct as-is, and the owned-patch plan discussed offline is dropped, since there's nothing to patch.

The one real gap this surfaced

Not a defect in the sync, but worth tracking: which module to import is not guessable from the call site. "42".toNat() needs import Nat, bytes.toBlob() needs Array (toBlob : (self : [Nat8]) -> Blob is defined there). An agent whose instinct on failure is import Text gets nowhere.

--implicit-package=core removes the requirement — moc: "allow contextual dot and implicits resolution from all modules in the given package" — and our caffeine-app skill sets it in its mops.toml, but writing-motoko/references/project-setup.md, whose entire job is the one-time [moc] args, doesn't mention it. So a self-managed project set up from our own skill hits the import requirement that a Caffeine project never sees.

Raised upstream on #10 with an offer to PR it. Not blocking this sync.

@raymondk
raymondk merged commit 4fb5e4e into main Sep 1, 2026
7 checks passed
@raymondk
raymondk deleted the chore/sync-upstream-skills-38a1136 branch September 1, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants