Skip to content

Commit fc0e709

Browse files
madikarizmarunebCopilot
authored
fix: correct buggy example code in skills, standardize CLI command form (#55)
* fix: correct buggy example code in skill references Several reference examples contain code an agent would copy verbatim but that doesn't behave as intended: content-experimentation-best-practices/references/cms-integration.md - assignVariant drew Math.random() * 100 without normalizing against the sum of variant weights, so weights that don't total 100 skew the split - it reused a cached cookie value without checking it's still a valid variant, leaving users bucketed to removed/renamed IDs - it assumed a non-empty variants array (variants[length - 1] throws on []) - the GROQ example filtered experimentVariants only by experiment status, so the first running experiment won regardless of the user's assignment; now scoped by experimentId plus the assigned variantId sanity-best-practices/references/migration-html-import.md - the custom <a> deserializer returned a top-level _type: 'link' block; htmlToBlocks expects an __annotation with a markDef and children from next(), matching portable-text-conversion/rules/html-to-pt.md sanity-best-practices/references/functions.md - the auto-tag function listened on create/update and wrote to tags with no guard, re-triggering itself in a loop; added !defined(tags) to the filter, mirroring the recursion guard in the translation example * fix: more buggy example code in skill references Second batch of fixes from automated review (Cursor Bugbot): - cms-integration.md: the assignVariant fallback returned the last variant without calling setCookie, so visitors who hit that path (stale/invalid cookie, zero total weight) were re-bucketed on every request; persist it - functions.md: the step-by-step 'Creating a Function' tutorial filtered on _type == "post" while its handler patches the same document — add the !defined(firstPublished) recursion guard the file itself prescribes - angular.md: the Transfer State snippet had literal '+' diff markers pasted in, making it invalid TypeScript; strip them and hoist the imports * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: address Copilot review findings in skill examples - migration-html-import: guard `el.tagName` with optional chaining so the custom rules fall through for non-element nodes (text nodes) instead of throwing, matching portable-text-conversion/rules/html-to-pt.md. - angular: widen the TransferState key to `ClientReturn<Query> | null` so `get(key, null)` type-checks against Angular's `get<T>(key: StateKey<T>, defaultValue: T): T`. * fix: guard null href/src in htmlToBlocks deserializer examples `getAttribute` returns null for a missing attribute, so the examples could emit `href: null` markDefs and the literal string `image@null` — the latter only fails later at asset-upload time, with no pointer back to the bad node. Both rules now read the attribute once and fall through when it is absent, so the surrounding text is still deserialized by the default handling. Applies the same fix to portable-text-conversion/rules/html-to-pt.md, which carried the identical `image@null` bug. * docs: scope _sanityAsset examples to the NDJSON import path `_sanityAsset` is resolved by the CLI importer, which fetches each `image@<url>` and swaps in a real asset reference. The mutation API does not interpret it, so the same blocks written via @sanity/client, `sanity exec`, or `defineMigration` are stored verbatim, leaving an image field with no `asset` reference. Neither example said so, and migration-html-import.md labelled the directive "Upload image separately, store reference" — a description of the client-upload path it is not. It then went on to demonstrate `defineMigration`, the very path where the directive is inert. Name the supported path at both call sites, add a note pointing at the upload-first alternative, and flag the constraint in the `defineMigration` section. Refs: https://www.sanity.io/docs/content-lake/importing-data * docs: use canonical plural CLI topics throughout The skills used both forms: `sanity dataset import` / `schema deploy` / `migration run` alongside `blueprints *` / `functions *`. Standardize on the plural form. Plural is the canonical topic in @sanity/cli 7.x — registered in oclif.config.js and backed by dist/commands/<topic>/, so oclif dispatches it directly. Singular resolves only by missing oclif lookup and landing in the command_not_found hook, which rewrites it via topicAliases. That shim works today and is silent, but it is a compatibility path, and it is not what the docs show. Covers dataset -> datasets, schema -> schemas, migration -> migrations. `blueprints` and `functions` were already plural. --------- Co-authored-by: Rune Botten <rbotten@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent fc8116b commit fc0e709

15 files changed

Lines changed: 133 additions & 70 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ This is a Sanity-powered project. Use the Knowledge Router below to find Sanity
99
npx sanity@latest mcp configure # Configure MCP for your AI editor
1010

1111
# Schema & Types
12-
npx sanity schema deploy # Deploy schema to Content Lake for MCP/editor access
13-
npx sanity schema extract # Extract schema for TypeGen
12+
npx sanity schemas deploy # Deploy schema to Content Lake for MCP/editor access
13+
npx sanity schemas extract # Extract schema for TypeGen
1414
npx sanity typegen generate # Generate TypeScript types
1515

1616
# Development

commands/deploy-schema.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Schema deployment is **required** for:
1818
## Deployment Command
1919

2020
```bash
21-
npx sanity schema deploy
21+
npx sanity schemas deploy
2222
```
2323

2424
## What I'll Do
@@ -29,7 +29,7 @@ npx sanity schema deploy
2929
- Warn about removed fields with data
3030

3131
2. **Deploy**
32-
- Run `npx sanity schema deploy`
32+
- Run `npx sanity schemas deploy`
3333
- Confirm success
3434

3535
3. **Post-Deploy Verification**

commands/typegen.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ I'll help you generate TypeScript types from your Sanity schema.
1212
```bash
1313
npm run typegen
1414
# or
15-
npx sanity schema extract && npx sanity typegen generate
15+
npx sanity schemas extract && npx sanity typegen generate
1616
```
1717

1818
## What I'll Do

skills/content-experimentation-best-practices/references/cms-integration.md

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -133,33 +133,60 @@ defineField({
133133
### 3. Frontend Assignment
134134

135135
```typescript
136-
// Middleware or server-side
137-
function assignVariant(experimentId: string, variants: Variant[]): string {
138-
// Check for existing assignment in cookie
136+
// Middleware or server-side. Returns the assigned variant id, or null when the
137+
// experiment has no variants to assign.
138+
function assignVariant(experimentId: string, variants: Variant[]): string | null {
139+
if (variants.length === 0) return null
140+
141+
// Reuse an existing assignment, but only if it's still a valid variant.
142+
// After variants are renamed or removed, drop the stale cookie and reassign,
143+
// otherwise users stay bucketed to IDs that no longer exist.
139144
const cookieKey = `exp_${experimentId}`
140145
const existing = getCookie(cookieKey)
141-
if (existing) return existing
142-
143-
// Random assignment based on weights
144-
const rand = Math.random() * 100
146+
if (existing && variants.some(v => v.id === existing)) return existing
147+
148+
// Random assignment based on weights. Normalize against the total weight so
149+
// splits work even when weights don't sum to 100 (otherwise draws above the
150+
// sum skew to the fallback).
151+
const totalWeight = variants.reduce((sum, v) => sum + v.weight, 0)
152+
if (totalWeight <= 0) {
153+
const fallback = variants[0]
154+
setCookie(cookieKey, fallback.id, { maxAge: 30 * 24 * 60 * 60 })
155+
return fallback.id
156+
}
157+
const rand = Math.random() * totalWeight
145158
let cumulative = 0
146159
for (const variant of variants) {
147160
cumulative += variant.weight
148-
if (rand <= cumulative) {
161+
if (rand < cumulative) {
149162
setCookie(cookieKey, variant.id, { maxAge: 30 * 24 * 60 * 60 })
150163
return variant.id
151164
}
152165
}
153-
return variants[0].id
166+
// Fall back to the last variant to absorb any floating-point remainder, and
167+
// persist it like any other assignment so the visitor stays bucketed.
168+
const fallback = variants[variants.length - 1]
169+
setCookie(cookieKey, fallback.id, { maxAge: 30 * 24 * 60 * 60 })
170+
return fallback.id
154171
}
155172
```
156173

157174
### 4. Query with Variant
158175

176+
Resolve **one** experiment's assignment at a time, passing both its
177+
`experimentId` and the `variantId` returned by `assignVariant`. Match on both:
178+
variant IDs like `control` repeat across experiments, so filtering on
179+
`variantId` alone could pick a different running experiment's row. For a page
180+
running several experiments, resolve each one separately and merge the results.
181+
159182
```groq
160183
*[_type == "page" && slug.current == $slug][0]{
161184
...,
162-
"experiment": experimentVariants[experimentId->status == "running"][0]{
185+
"experiment": experimentVariants[
186+
experimentId->_id == $experimentId &&
187+
experimentId->status == "running" &&
188+
variantId == $variantId
189+
][0]{
163190
experimentId->{name, _id},
164191
variantId,
165192
headline

skills/portable-text-conversion/rules/html-to-pt.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,19 @@ const blocks = htmlToBlocks(html, blockContentType, {
114114
deserialize(el, next, block) {
115115
if (el.tagName?.toLowerCase() !== 'img') return undefined
116116

117+
const src = el.getAttribute('src')
118+
if (!src) return undefined // skip sourceless images, not `image@null`
119+
117120
return block({
118121
_type: 'image',
119122
asset: {
120123
_type: 'reference',
121124
_ref: '', // Upload image separately, set ref after
122125
},
123126
alt: el.getAttribute('alt') || '',
124-
_sanityAsset: `image@${el.getAttribute('src')}`, // for migration tooling
127+
// Resolved by `sanity datasets import` only. On client/mutation-API
128+
// write paths, upload the asset first and set `asset._ref` instead.
129+
_sanityAsset: `image@${src}`,
125130
})
126131
},
127132
},
@@ -233,7 +238,7 @@ export default defineMigration({
233238
})
234239
```
235240

236-
Run with: `sanity migration run import-wordpress-posts --no-dry-run`
241+
Run with: `sanity migrations run import-wordpress-posts --no-dry-run`
237242

238243
## Reference
239244

skills/sanity-best-practices/references/angular.md

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -442,39 +442,40 @@ Key considerations for Sanity + Angular SSR:
442442
Angular's built-in HTTP Transfer Cache does not cover `@sanity/client` requests. Without manual transfer, the client re-fetches every query during hydration. Add `TransferState` to the service from Section 2:
443443

444444
```typescript
445-
+ async function hashQuery(query: string, params?: QueryParams): Promise<string> {
446-
+ const input = query + JSON.stringify(params ?? {})
447-
+ const buffer = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(input))
448-
+ return Array.from(new Uint8Array(buffer), b => b.toString(16).padStart(2, '0')).join('')
449-
+ }
450-
451-
import { Injectable, inject } from '@angular/core'
452-
+ import { isPlatformBrowser, isPlatformServer } from '@angular/common'
453-
+ import { PLATFORM_ID, makeStateKey, TransferState } from '@angular/core'
445+
import { Injectable, inject, PLATFORM_ID, makeStateKey, TransferState } from '@angular/core'
446+
import { isPlatformBrowser, isPlatformServer } from '@angular/common'
454447
import { createClient, type ClientReturn, type QueryParams, type SanityClient } from '@sanity/client'
455448

449+
async function hashQuery(query: string, params?: QueryParams): Promise<string> {
450+
const input = query + JSON.stringify(params ?? {})
451+
const buffer = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(input))
452+
return Array.from(new Uint8Array(buffer), b => b.toString(16).padStart(2, '0')).join('')
453+
}
454+
456455
export class SanityService {
457456
private client: SanityClient
458-
+ private transferState = inject(TransferState)
459-
+ private platformId = inject(PLATFORM_ID)
457+
private transferState = inject(TransferState)
458+
private platformId = inject(PLATFORM_ID)
460459

461460
async fetch<Query extends string>(query: Query, params?: QueryParams): Promise<ClientReturn<Query>> {
462-
+ const key = makeStateKey<ClientReturn<Query>>(await hashQuery(query, params))
463-
+
464-
+ if (isPlatformBrowser(this.platformId)) {
465-
+ const cached = this.transferState.get(key, null)
466-
+ if (cached !== null) {
467-
+ this.transferState.remove(key)
468-
+ return cached
469-
+ }
470-
+ }
471-
+
461+
// The key type includes `null` so `get(key, null)` type-checks against
462+
// Angular's `get<T>(key: StateKey<T>, defaultValue: T): T` signature.
463+
const key = makeStateKey<ClientReturn<Query> | null>(await hashQuery(query, params))
464+
465+
if (isPlatformBrowser(this.platformId)) {
466+
const cached = this.transferState.get(key, null)
467+
if (cached !== null) {
468+
this.transferState.remove(key)
469+
return cached
470+
}
471+
}
472+
472473
const result = await this.client.fetch(query, params)
473-
+
474-
+ if (isPlatformServer(this.platformId)) {
475-
+ this.transferState.set(key, result)
476-
+ }
477-
+
474+
475+
if (isPlatformServer(this.platformId)) {
476+
this.transferState.set(key, result)
477+
}
478+
478479
return result
479480
}
480481
}

skills/sanity-best-practices/references/functions.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,10 @@ export default defineBlueprint({
9494
name: 'my-function',
9595
event: {
9696
on: ['create', 'update'],
97-
filter: '_type == "post"',
97+
// The handler patches the same document, which emits another update
98+
// event. Guard with !defined(firstPublished) so the function stops
99+
// matching once it has run — see "Recursion control" below.
100+
filter: '_type == "post" && !defined(firstPublished)',
98101
},
99102
}),
100103
],
@@ -460,7 +463,10 @@ defineDocumentFunction({
460463
name: 'auto-tag',
461464
event: {
462465
on: ['create', 'update'],
463-
filter: "_type == 'post'",
466+
// Only fire while tags are missing. The handler writes to `tags`, which
467+
// emits another `update` event — without this guard the function would
468+
// re-trigger itself in a loop. Once tags exist, the filter stops matching.
469+
filter: "_type == 'post' && !defined(tags)",
464470
projection: '{_id, title, body}',
465471
},
466472
})

skills/sanity-best-practices/references/get-started.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export const post = defineType({
9797
**Required before Phase 2:**
9898

9999
```bash
100-
npx sanity schema deploy
100+
npx sanity schemas deploy
101101
```
102102

103103
This uploads your schema to the Content Lake so MCP tools can work with it.
@@ -137,7 +137,7 @@ Tool: create_documents
137137
Documents: [{ type: "post", content: { title: "Getting started with Sanity", body: [] } }]
138138
```
139139

140-
**If MCP content tools cannot see new types or fields:** Remind them to run `npx sanity schema deploy` first.
140+
**If MCP content tools cannot see new types or fields:** Remind them to run `npx sanity schemas deploy` first.
141141

142142
### MCP Setup (If Not Configured)
143143

@@ -385,7 +385,7 @@ Just ask about any of these!"
385385
```bash
386386
npx sanity@latest mcp configure # Configure MCP for your editor
387387
npx sanity dev # Start Studio locally
388-
npx sanity schema deploy # Deploy schema for MCP/editor access
388+
npx sanity schemas deploy # Deploy schema for MCP/editor access
389389
npx sanity deploy # Deploy Studio to Sanity hosting
390390
npx sanity manage # Open project settings
391391
npm run typegen # Generate TypeScript types

skills/sanity-best-practices/references/migration-html-import.md

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,35 @@ const blocks = htmlToBlocks(htmlString, blockContentType, {
3737
rules: [
3838
{
3939
deserialize(el, next, block) {
40-
// Custom link handling
41-
if (el.tagName.toLowerCase() === 'a') {
40+
// Custom link handling — links are inline annotations, not blocks.
41+
// Return an `__annotation` with a `markDef`, and recurse into the
42+
// child nodes via `next()` so the link text is preserved.
43+
if (el.tagName?.toLowerCase() === 'a') {
44+
const href = el.getAttribute('href')
45+
// An anchor with no `href` (named anchors, JS-driven links) isn't a
46+
// link. Fall through so the text survives without a dangling markDef.
47+
if (!href) return undefined
4248
return {
43-
_type: 'link',
44-
href: el.getAttribute('href'),
45-
blank: el.getAttribute('target') === '_blank'
49+
_type: '__annotation',
50+
markDef: {
51+
_type: 'link',
52+
href,
53+
blank: el.getAttribute('target') === '_blank'
54+
},
55+
children: next(el.childNodes)
4656
}
4757
}
48-
// Custom image handling
49-
if (el.tagName.toLowerCase() === 'img') {
50-
return {
58+
// Custom image handling — block-level types are wrapped with `block()`
59+
if (el.tagName?.toLowerCase() === 'img') {
60+
const src = el.getAttribute('src')
61+
// Skip sourceless images rather than emitting `image@null`, which
62+
// the importer reports as a failed asset with no pointer to the node.
63+
if (!src) return undefined
64+
return block({
5165
_type: 'image',
52-
// Upload image separately, store reference
53-
_sanityAsset: `image@${el.getAttribute('src')}`
54-
}
66+
// NDJSON + `sanity datasets import` only — see the note below.
67+
_sanityAsset: `image@${src}`
68+
})
5569
}
5670
return undefined // Fall through to default handling
5771
}
@@ -60,6 +74,14 @@ const blocks = htmlToBlocks(htmlString, blockContentType, {
6074
})
6175
```
6276
77+
> **`_sanityAsset` is only resolved by `sanity datasets import`.** The NDJSON
78+
> importer fetches each `image@<url>` and swaps in a real asset reference. The
79+
> mutation API does not interpret the directive, so the same blocks written
80+
> through `@sanity/client`, `sanity exec`, or `defineMigration` are stored
81+
> verbatim — leaving an image field with a stray `_sanityAsset` string and no
82+
> `asset` reference. On those paths, upload the image first and emit an asset
83+
> reference instead, as in [Image Upload](#image-upload) below.
84+
6385
### Pre-Processing HTML
6486
6587
Clean HTML before conversion:
@@ -105,7 +127,9 @@ async function uploadImage(client, imageUrl) {
105127
106128
### Using in a Migration
107129
108-
Wrap this in `defineMigration` for controlled imports:
130+
Wrap this in `defineMigration` for controlled imports. This path writes through
131+
the mutation API, so any custom rules used here must emit uploaded asset
132+
references rather than `_sanityAsset` directives:
109133
110134
```typescript
111135
// migrations/import-wordpress-posts/index.ts
@@ -136,6 +160,6 @@ export default defineMigration({
136160
137161
Let Sanity generate document IDs for ordinary imported content. Add schema fields for legacy identifiers or slugs, then use GROQ lookups against those fields when you need to rerun an import, patch existing documents, or create references between imported records. Set `_id` directly only for singleton documents.
138162
139-
Run with: `sanity migration run import-wordpress-posts --no-dry-run`
163+
Run with: `sanity migrations run import-wordpress-posts --no-dry-run`
140164
141165
Reference: [Schema and Content Migrations](https://www.sanity.io/docs/content-lake/schema-and-content-migrations)

skills/sanity-best-practices/references/schema.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,10 +313,10 @@ export default defineMigration({
313313

314314
```bash
315315
# Dry run first (default)
316-
sanity migration run rename-oldTitle-to-newTitle
316+
sanity migrations run rename-oldTitle-to-newTitle
317317

318318
# Execute when ready
319-
sanity migration run rename-oldTitle-to-newTitle --no-dry-run
319+
sanity migrations run rename-oldTitle-to-newTitle --no-dry-run
320320
```
321321

322322
**Phase 3: Remove** — Once `oldTitle` is undefined for all documents, delete the field definition.

0 commit comments

Comments
 (0)