-
Notifications
You must be signed in to change notification settings - Fork 11
Update setup flow for people who just want to get a live rendered pat… #1980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 4 files
.claude/commands/setup.md
Outdated
| Check if ct binary exists: | ||
| ```bash | ||
| ls -la ./dist/ct | ||
| ``` | ||
|
|
||
| If it doesn't exist, build it: | ||
| ```bash | ||
| deno task build-binaries --cli-only | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should simply reference the "ct" skill here, since it explains building vs using deno task ct already.
.claude/commands/setup.md
Outdated
| **3. Deploy another pattern:** | ||
| Try deploying a different pattern from packages/patterns/: | ||
|
|
||
| ```bash | ||
| ./dist/ct charm new --identity claude.key --api-url http://localhost:8000 --space test-space packages/patterns/dice.tsx | ||
| ``` | ||
|
|
||
| Then visit: http://localhost:5173/test-space/[NEW_CHARM_ID] | ||
|
|
||
| **4. Modify a pattern:** | ||
| Edit packages/patterns/ct-checkbox-cell.tsx, then update: | ||
|
|
||
| ```bash | ||
| ./dist/ct charm setsrc --identity claude.key --api-url http://localhost:8000 --space test-space --charm [CHARM_ID] packages/patterns/ct-checkbox-cell.tsx | ||
| ``` | ||
|
|
||
| Refresh your browser to see changes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is somewhat duplicating the content in the pattern-dev skill, so we can probably just reference that (pattern-dev contains workflow-guide.md which explains this section)
bfollington
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM just a couple places we could reference the skill / docs.
Can probably ask Claude to read over the docs and skills and remove the duplicated content based on my comments. (You can feed the PR review in via gh CLI app and PR URL)
…1919) * Support `default` as sibling to `$ref` in JSON schemas ## Summary Implements full support for the `default` keyword as a sibling to `$ref` in JSON schemas, per JSON Schema 2020-12 specification. This allows schema definitions to be reused via `$ref` while overriding default values at the reference site. ## Motivation JSON Schema 2020-12 permits siblings next to `$ref` (a change from earlier specs), with the ref site properties taking precedence. Previously, our codebase only partially supported `asCell` and `asStream` as siblings to `$ref`. This commit extends that support to `default` and fixes gaps in the existing sibling handling. ## Changes ### Core Schema Resolution (schema.ts) **`resolveSchema()` [lines 47-140]** - Captures ref site siblings (`default`, `asCell`, `asStream`) before resolving the `$ref` - Applies ref site siblings to resolved schema (ref site overrides target) - Handles boolean schema targets by converting `true` to object when siblings present - Properly handles `false` schema (cannot be augmented) ### Schema Path Walking (cfc.ts) **`schemaAtPath()` [lines 469-516]** - Mirrors `resolveSchema()` sibling preservation pattern - Captures and applies ref site `default`, `asCell`, `asStream` - Fixes existing gap: previously only preserved `ifc` tags - Now correctly handles all sibling types when walking schema paths ### Test Coverage (schema-ref-default.test.ts) Added 19 comprehensive test cases covering: - Basic precedence (ref site default overrides target default) - Chained refs with defaults at multiple levels - Boolean schema targets (`true`/`false`) with defaults - Interaction with `asCell` and `asStream` - `anyOf`/`oneOf` with refs having defaults - Various default value types (primitives, objects, arrays, null) - Edge cases with `filterAsCell` flag ## Precedence Rules 1. **Ref site default ALWAYS wins over target default** ```json { "$ref": "#/target", "default": "WINS" } // target: { "type": "string", "default": "loses" } // result: { "type": "string", "default": "WINS" } ``` 2. **In chained refs, outermost ref site default applies** ```json { "$ref": "#/A", "default": "outer" } // A: { "$ref": "#/B", "default": "middle" } // B: { "type": "string", "default": "inner" } // result: { "type": "string", "default": "outer" } ``` 3. **`default` is NOT filtered** (unlike `asCell`/`asStream` with `filterAsCell=true`) 4. **Target default preserved when no ref site default exists** ## Implementation Notes **Why phases 2, 4, 5 from the plan weren't needed:** - `processDefaultValue()` uses `resolvedSchema.default`, which now includes merged ref site default - `validateAndTransform()` default checks automatically get correct precedence - `anyOf`/`oneOf` handling calls `resolveSchema()` on each option, which handles precedence **Existing functionality preserved:** - All 73 existing runner tests pass (975 test steps) - `asCell`/`asStream` filtering behavior unchanged - IFC tag collection still works correctly - Boolean schema handling improved ## Testing ✅ 19 new tests pass (comprehensive `default` sibling support) ✅ 73 existing runner tests pass (975 steps, no regressions) ✅ Type checking passes ✅ Code formatted with `deno fmt` ## Examples **Basic usage:** ```typescript const schema = { $defs: { Email: { type: "string", format: "email" } }, properties: { userEmail: { $ref: "#/$defs/Email", default: "[email protected]" // ← Overrides target default } } }; ``` **With asCell:** ```typescript const schema = { $defs: { Counter: { type: "number" } }, properties: { count: { $ref: "#/$defs/Counter", default: 0, asCell: true // ← Both siblings preserved } } }; ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Add type-level $ref resolution with JSON Pointer support to Schema<> types ## Summary Implements full type-level support for JSON Schema $ref resolution, including JSON Pointer paths like `#/$defs/Address` and `#/properties/name`. This brings the TypeScript type system in parity with the runtime schema resolution while preserving proper handling of `default` values and ref site sibling merging. ## Motivation Previously, `Schema<>` and `SchemaWithoutCell<>` only handled `$ref: "#"` (self- reference). Any other `$ref` returned `any`, losing type information. With JSON Pointer support, TypeScript can now resolve refs to definitions in `$defs`, properties, and other schema locations at compile time. ## Changes ### Type System Utilities (api/index.ts) **Path Parsing & Navigation (lines 608-670):** - `SplitPath<>`: Parse JSON Pointer strings into path segments - `SplitPathSegments<>`: Recursively split path on `/` delimiters - `NavigatePath<>`: Walk schema tree following a path array - `ResolveRef<>`: Resolve `$ref` string to target schema **Schema Merging (lines 672-720):** - `MergeSchemas<>`: Merge ref site siblings with target schema - `MergeRefSiteWithTarget<>`: Merge then process with `Schema<>` - `MergeRefSiteWithTargetWithoutCell<>`: Same for `SchemaWithoutCell<>` Implements JSON Schema 2020-12 spec: ref site siblings (like `default`, `asCell`, `asStream`) override corresponding properties in the target schema. ### Schema<> Type Updates **$ref Resolution (lines 749-756):** ```typescript // OLD: Only handled "#" refs : T extends { $ref: "#" } ? Schema<Omit<Root, "asCell" | "asStream">, ...> : T extends { $ref: string } ? any // Everything else returned any // NEW: Full JSON Pointer support : T extends { $ref: infer RefStr extends string } ? MergeRefSiteWithTarget<T, ResolveRef<RefStr, ...>, ...> ``` **Benefits:** - `#/$defs/Address` resolves to actual type - `#/properties/name` creates type alias - Ref site `default` overrides target `default` (JSON Schema 2020-12 spec) - Chained refs resolve correctly (A→B→C) - Works with `asCell`/`asStream` siblings ### Test Coverage (schema-to-ts.test.ts) Added 11 comprehensive test cases in new describe block: - Basic `$ref` to `$defs` resolution - Default value precedence (ref site overrides target) - Nested refs through multiple levels - Chained refs (A→B→C resolves to C's type) - Refs in array items - Refs in `anyOf` branches - Refs with `asCell` wrapper - Complex nested structures with refs - Multiple refs to same definition - Refs to properties paths ## Type System Capabilities **Supported:** - `#` (self-reference to root) - `#/$defs/Name` (definition references) - `#/properties/field` (property references) - Any JSON Pointer path within the document - Default precedence (ref site wins per JSON Schema spec) - Sibling merging (asCell, asStream, default, etc.) **Limitations:** - JSON Pointer escaping (~0, ~1) not supported at type level - External refs (http://...) return `any` - Depth limited to 9 levels for recursion safety - Properties with `default` remain optional (correct per JSON Schema spec) ## Precedence Rules (Matches Runtime) 1. **Ref site siblings override target**: `{ $ref: "#/foo", default: "bar" }` 2. **Chained refs use outermost**: Only the original ref site siblings apply 3. **Properties with default are optional**: Type system matches JSON Schema spec 4. **Runtime fills in defaults**: Properties have fallback values at runtime ## Testing ✅ All 73 runner tests pass (987 steps, 0 failed) ✅ All 24 schema-to-ts tests pass (11 new + 13 existing) ✅ All 19 schema-ref-default tests pass (runtime $ref with default) ✅ Type checking: 3 pre-existing errors unchanged (in schema.test.ts) ✅ No new type errors introduced ✅ No breaking changes to existing code ## Examples **Basic ref to definition:** ```typescript type MySchema = Schema<{ $defs: { Email: { type: "string"; format: "email" }; }; properties: { contact: { $ref: "#/$defs/Email" }; }; }>; // Result: { contact?: string } ``` **Ref with default override:** ```typescript type MySchema = Schema<{ $defs: { Counter: { type: "number"; default: 0 }; }; properties: { score: { $ref: "#/$defs/Counter"; default: 100 }; }; }>; // Result: { score?: number } // At runtime, score defaults to 100 (ref site wins), not 0 ``` **Chained refs:** ```typescript type MySchema = Schema<{ $defs: { C: { type: "string" }; B: { $ref: "#/$defs/C" }; A: { $ref: "#/$defs/B" }; }; properties: { value: { $ref: "#/$defs/A" }; }; }>; // Result: { value?: string } - fully resolved through chain ``` **Ref with asCell:** ```typescript type MySchema = Schema<{ $defs: { Config: { type: "object"; properties: { theme: { type: "string" } } }; }; properties: { settings: { $ref: "#/$defs/Config"; asCell: true }; }; }>; // Result: { settings?: Cell<{ theme?: string }> } ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Fix self-referential $ref type resolution in Schema<> types ## Problem The previous implementation of JSON Pointer-based $ref resolution broke type inference for self-referential schemas using `$ref: "#"`. This caused 3 type errors in schema.test.ts: 1. Line 706: Property 'id' missing on nested Cell with `$ref: "#"` 2. Lines 978-979: Properties possibly undefined on recursive LinkedNode ## Root Cause The generic $ref handler was processing `$ref: "#"` through MergeRefSiteWithTarget, which created a new merged object type. This broke the recursive type structure that TypeScript needs for proper inference of self-referential schemas. ## Solution 1. **Special-case `$ref: "#"` before general `$ref` handler**: Added explicit checks for self-references in both Schema<> and SchemaWithoutCell<> types to preserve the original recursive type resolution behavior. 2. **Add non-null assertions to LinkedNode test**: Updated test expectations to reflect that `next` is correctly typed as optional per the schema definition. The old implementation typed non-"#" refs as `any`, hiding this issue. ## Changes - packages/api/index.ts: - Schema<>: Check for `$ref: "#"` before generic `$ref` handler - SchemaWithoutCell<>: Same pattern for non-Cell variant - packages/runner/test/schema.test.ts: - Add `!` assertions when accessing optional `next` property ## Testing - ✅ `deno task check` passes (0 type errors) - ✅ All runtime tests pass (73 passed, 0 failed) - ✅ All schema-to-ts tests pass (24 steps) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Enhance the Schema type to handle refs to $defs/Root * Updated SchemaWithoutCell utility Properly rebuilt api types * Move this file to the right place * Fixed import path Resolved schemas should have $defs still, so we can use these without a root. Added test for a default that's one step from the top Altered tests, since resolved schema should not have a top level $ref A false schema should be converted into an object with `"not": true`, since we may need to preserve other schema properties. * Remove the "cannot add default" note for the false schema test --------- Co-authored-by: Bernhard Seefeld <[email protected]> Co-authored-by: Claude <[email protected]>
* Fix map closure destructured alias handling and add regression fixtures * Fix map closure capture name collisions and add regression fixture * Avoid recomputing computed destructured aliases and add fixture
…es (#1973) * Fix map closure destructured alias handling and add regression fixtures * Avoid recomputing destructured computed keys and restore numeric-key fixture coverage * fix(transform): preserve directive prologues when prepending computed initializers * fix: properly deduplicate captured variables with unique suffixes
* Update the omnibot to have a star trek computer style. Also encourage it to search within the space via tools before going to searchWeb and external resources. * "Prefer action to explanation" I think this will be better than a negative instruction. * Format pass --------- Co-authored-by: Ben Follington <[email protected]>
* WIP tx-analysis * Add real implementation * Remove test data and scripts * Refine tx summary * Remove long pointless README * Remove accidental `mise.toml` * Remove JournalMock from tests * Remove unused analysis * Format and lint * PR feedback
* Auto-attach currently viewed charm and introduce `listRecent` tool * Make system prompt customizable for `chatbot.tsx`
- Reference ct skill for binary building instead of duplicating commands - Reference pattern-dev skill's workflow-guide.md for deployment/modification workflows - Removes duplication as requested in review comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed changes from recent commits (found 3 issues).
3 issues found across 29 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="packages/ts-transformers/test/fixtures/closures/map-destructured-numeric-alias.input.tsx">
<violation number="1" location="packages/ts-transformers/test/fixtures/closures/map-destructured-numeric-alias.input.tsx:13">
Each element returned from this map needs a stable key to keep reconciliation safe; add a key to the <span> rendered inside the map.</violation>
</file>
<file name="packages/patterns/chatbot.tsx">
<violation number="1" location="packages/patterns/chatbot.tsx:459">
The auto-attached recent charm can no longer be removed: the bar renders attachmentsWithRecent, but onct-remove still mutates allAttachments, which never contains the injected item, so the remove action silently fails.</violation>
</file>
<file name="packages/ts-transformers/test/fixtures/closures/map-computed-alias-side-effect.input.tsx">
<violation number="1" location="packages/ts-transformers/test/fixtures/closures/map-computed-alias-side-effect.input.tsx:4">
`keyCounter` is module-scoped, so every re-render increments it past the original property names; `nextKey()` then requests keys the objects do not have, so `amount` becomes `undefined` on subsequent renders. Move the counter inside the recipe (or reset it before each map) so it starts from zero per render.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| [UI]: ( | ||
| <div> | ||
| {state.entries.map(({ 0: first }) => ( | ||
| <span>{first}</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each element returned from this map needs a stable key to keep reconciliation safe; add a key to the rendered inside the map.
Prompt for AI agents
Address the following comment on packages/ts-transformers/test/fixtures/closures/map-destructured-numeric-alias.input.tsx at line 13:
<comment>Each element returned from this map needs a stable key to keep reconciliation safe; add a key to the <span> rendered inside the map.</comment>
<file context>
@@ -0,0 +1,18 @@
+ [UI]: (
+ <div>
+ {state.entries.map(({ 0: first }) => (
+ <span>{first}</span>
+ ))}
+ </div>
</file context>
| <ct-hstack align="center" gap="1"> | ||
| <ct-attachments-bar | ||
| attachments={allAttachments} | ||
| attachments={attachmentsWithRecent} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auto-attached recent charm can no longer be removed: the bar renders attachmentsWithRecent, but onct-remove still mutates allAttachments, which never contains the injected item, so the remove action silently fails.
Prompt for AI agents
Address the following comment on packages/patterns/chatbot.tsx at line 459:
<comment>The auto-attached recent charm can no longer be removed: the bar renders attachmentsWithRecent, but onct-remove still mutates allAttachments, which never contains the injected item, so the remove action silently fails.</comment>
<file context>
@@ -388,7 +456,7 @@ export default recipe<ChatInput, ChatOutput>(
<ct-hstack align="center" gap="1">
<ct-attachments-bar
- attachments={allAttachments}
+ attachments={attachmentsWithRecent}
removable
onct-remove={removeAttachment({ allAttachments })}
</file context>
| /// <cts-enable /> | ||
| import { recipe, UI } from "commontools"; | ||
|
|
||
| let keyCounter = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keyCounter is module-scoped, so every re-render increments it past the original property names; nextKey() then requests keys the objects do not have, so amount becomes undefined on subsequent renders. Move the counter inside the recipe (or reset it before each map) so it starts from zero per render.
Prompt for AI agents
Address the following comment on packages/ts-transformers/test/fixtures/closures/map-computed-alias-side-effect.input.tsx at line 4:
<comment>`keyCounter` is module-scoped, so every re-render increments it past the original property names; `nextKey()` then requests keys the objects do not have, so `amount` becomes `undefined` on subsequent renders. Move the counter inside the recipe (or reset it before each map) so it starts from zero per render.</comment>
<file context>
@@ -0,0 +1,23 @@
+/// <cts-enable />
+import { recipe, UI } from "commontools";
+
+let keyCounter = 0;
+function nextKey() {
+ return `value-${keyCounter++}`;
</file context>
…tern on screen.
Summary by cubic
Introduces a one-command setup to get a live pattern rendering fast with only Deno. Also improves schema typing, chatbot UX, and tool-call summaries to smooth early development.
New Features
/setupcommand anddocs/QUICKSTART.mdto check prerequisites, buildct, create an identity, start local servers, deploy a demo, and share the URL and next steps.listRecent, and supports a customizable system prompt; Omnibox gets a concise “Star Trek computer” style prompt.Schema<>andSchemaWithoutCell<>resolve JSON Pointer$ref(e.g.,#/$defs/...,#/properties/...) and correctly merge ref-site defaults per spec.Bug Fixes
start-local-dev.shcan run from any directory.Written for commit 31b68b4. Summary will update automatically on new commits.