|
1 | 1 | # Repository Guidelines for AI Agents |
2 | 2 |
|
3 | | -The instructions in this document apply to the entire repository. |
| 3 | +## Recipe/Pattern Development |
4 | 4 |
|
5 | | -## Basics |
| 5 | +If you are developing recipes/patterns (they mean the same thing), read the following documentation: |
6 | 6 |
|
7 | | -### Build & Test |
| 7 | +- `docs/common/RECIPES.md` - Writing recipes with cells, handlers, lifts, best practices, and [ID] usage patterns |
| 8 | +- `docs/common/PATTERNS.md` - High-level patterns and examples for building applications, including common mistakes and debugging tips |
| 9 | +- `docs/common/HANDLERS.md` - Writing handler functions, event types, state management, and handler factory patterns |
| 10 | +- `docs/common/COMPONENTS.md` - Guide to UI components (ct-checkbox, ct-input, ct-select, etc.) with bidirectional binding and event handling patterns |
| 11 | +- `docs/common/DEVELOPMENT.md` - Coding style, design principles, and best practices |
| 12 | +- `docs/common/UI_TESTING.md` - How to work with shadow dom in our integration tests |
| 13 | +- `docs/common/RECIPE_DEPLOYMENT.md` - Deploying and testing recipes using the ct CLI tool |
8 | 14 |
|
9 | | -- Check typings with `deno task check`. |
10 | | -- Run all tests using `deno task test`. |
11 | | -- To run a single test file use `deno test path/to/test.ts`. |
12 | | -- To test a specific package, `cd` into the package directory and run |
13 | | - `deno task test`. |
| 15 | +Check `packages/patterns/` for working recipe examples. |
14 | 16 |
|
15 | | -### `ct` & Common Tools Framework |
| 17 | +**Important:** Ignore the top level `recipes` folder - it is defunct. |
16 | 18 |
|
17 | | -Before ever calling `ct` you MUST read `docs/common/CT.md`. |
| 19 | +## Runtime Development |
18 | 20 |
|
19 | | -### Recipe Development |
| 21 | +If you are developing runtime code, read the following documentation: |
20 | 22 |
|
21 | | -Whenever you work on patterns (sometimes called recipes), consult the `patterns` |
22 | | -package for a set of well-tested minimal examples. To learn more about the |
23 | | -pattern framework, consult `docs/common/*.md` and `tutorials/*.md`. You should |
24 | | -re-read `.claude/commands/recipe-dev.md` when confused to refresh your memory on |
25 | | -`ct` and deploying charms. |
26 | | - |
27 | | -These patterns are composed using functions from `packages/builder` and executed |
28 | | -by our runtime in `packages/runner`, managed by `packages/charm` and rendered by |
29 | | -`packages/html`. |
30 | | - |
31 | | -IMPORTANT: ignore the top level `recipes` folder, it is defunct. |
32 | | - |
33 | | -### Formatting |
34 | | - |
35 | | -- Line width is **80 characters**. |
36 | | -- Indent with **2 spaces**. |
37 | | -- **Semicolons are required.** |
38 | | -- Use **double quotes** for strings. |
39 | | -- Always run `deno fmt` before committing. |
40 | | - |
41 | | -### TypeScript |
42 | | - |
43 | | -- Export types explicitly using `export type { ... }`. |
44 | | -- Provide descriptive JSDoc comments on public interfaces. |
45 | | -- Prefer strong typing with interfaces or types instead of `any`. |
46 | | -- Update package-level README.md files. |
47 | | - |
48 | | -### Imports |
49 | | - |
50 | | -- Group imports by source: standard library, external, then internal. |
51 | | -- Prefer named exports over default exports. |
52 | | -- Use package names for internal imports. |
53 | | -- Destructure when importing multiple names from the same module. |
54 | | -- Import either from `@commontools/api` (internal API) or |
55 | | - `@commontools/api/interface` (external API), but not both. |
56 | | - |
57 | | -### Error Handling |
58 | | - |
59 | | -- Write descriptive error messages. |
60 | | -- Propagate errors using async/await. |
61 | | -- Document possible errors in JSDoc. |
62 | | - |
63 | | -### Testing |
64 | | - |
65 | | -- Structure tests with `@std/testing/bdd` (`describe`/`it`). |
66 | | -- Use `@std/expect` for assertions. |
67 | | -- Give tests descriptive names. |
68 | | -- Run using `deno task test` NOT `deno test`, the flags are important |
69 | | - |
70 | | -## Good Patterns & Practices |
71 | | - |
72 | | -Not all the code fits these patterns. For bigger changes, follow these |
73 | | -guidelines and consider refactoring existing code towards these practices. |
74 | | - |
75 | | -### Avoid Singletons |
76 | | - |
77 | | -The singleton pattern may be useful when there's a single global state. But |
78 | | -running multiple instances, unit tests, and reflecting state from another state |
79 | | -becomes impossible. Additionally, this pattern is infectious, often requiring |
80 | | -consuming code to also only support a single instance. |
81 | | - |
82 | | -> **❌ Avoid** |
83 | | -
|
84 | | -```ts |
85 | | -const cache = new Map(); |
86 | | -export const set = (key: string, value: string) => cache.set(key, value); |
87 | | -export const get = (key: string): string | undefined => cache.get(key); |
88 | | -``` |
89 | | - |
90 | | -```ts |
91 | | -export const cache = new Map(); |
92 | | -export const instance = new Foo(); |
93 | | -``` |
94 | | - |
95 | | -> **✅ Prefer** |
96 | | -
|
97 | | -In both cases, we can maintain multiple caches, or instances of cache consumers. |
98 | | - |
99 | | -```ts |
100 | | -export class Cache { |
101 | | - private map: Map<string, string> = new Map(); |
102 | | - get(key: string): string | undefined { |
103 | | - return this.map.get(key); |
104 | | - } |
105 | | - set(key: string, value: string) { |
106 | | - this.map.set(key, value); |
107 | | - } |
108 | | -} |
109 | | -``` |
110 | | - |
111 | | -Or with a functional pattern: |
112 | | - |
113 | | -```ts |
114 | | -export type Cache = Map; |
115 | | -export const get = (cache: Cache, key: string): string | undefined => |
116 | | - cache.get(key); |
117 | | -export const set = (cache: Cache, key: string, value: string) => |
118 | | - cache.set(key, value); |
119 | | -``` |
120 | | - |
121 | | -### Keep the Module Graph clean |
122 | | - |
123 | | -We execute our JavaScript modules in many different environments: |
124 | | - |
125 | | -- Browsers (Vite built) |
126 | | -- Browsers (deno-web-test>esbuild Built) |
127 | | -- Browsers (eval'd recipes) |
128 | | -- Deno (scripts and servers) |
129 | | -- Deno (eval'd recipes) |
130 | | -- Deno Workers |
131 | | -- Deno workers (eval'd recipes) |
132 | | - |
133 | | -Each frontend bundle or script has a single entry point[^1]. For frontend |
134 | | -bundles, it's a single JS file with every workspace/dependency module included. |
135 | | -For deno environments, the module graph is built dynamically. While JavaScript |
136 | | -can run in many environments, there's work to be done to run the same code |
137 | | -across all invocations. We should strive for a clear module graph for all |
138 | | -potential entries (e.g. each script, each bundle) for both portability, |
139 | | -maintanance, and performance. |
140 | | - |
141 | | -[^1]: Our vite frontend has multiple "pieces" for lazy loading JSDOM/TSC, but a |
142 | | - single "main". |
143 | | - |
144 | | -> **❌ Avoid** |
145 | | -
|
146 | | -- Modules depending on each other |
147 | | -- Large quantity of module exports |
148 | | -- Adding module-specific dependencies to workspace deno.json |
149 | | -- Non-standard JS (env vars, vite-isms): All of our different invocation |
150 | | - mechanisms/environments need to handle these |
151 | | - |
152 | | -> **✅ Prefer** |
153 | | -
|
154 | | -- Use |
155 | | - [manifest exports](https://docs.deno.com/runtime/fundamentals/workspaces/#multiple-package-entries) |
156 | | - to export a different entry point for a module. Don't pull in everything if |
157 | | - only e.g. types are needed. |
158 | | - - If needed, environment specific exports can be provided e.g. |
159 | | - `@workspace/module/browser` | `@workspace/module/deno`. |
160 | | -- Consider leaf nodes in the graph: A `utils` module should not be heavy with |
161 | | - dependencies, external or otherwise. |
162 | | -- Clean separation of public and private facing interfaces: only export what's |
163 | | - needed. |
164 | | -- Add module-specific dependencies to that module's dependencies, not the entire |
165 | | - workspace. We don't need `vite` in a deno server. |
166 | | - |
167 | | -### Avoid Ambiguous Types |
168 | | - |
169 | | -Softly-typed JS allows quite a bit. We often accept a range of inputs, and based |
170 | | -on type checking, perform actions. |
171 | | - |
172 | | -> **❌ Avoid** |
173 | | -
|
174 | | -Minimize unknown type usage. Not only does `processData` allow any type, but |
175 | | -it's unclear what the intended types are: |
176 | | - |
177 | | -```ts |
178 | | -function processData(data: any) { |
179 | | - if (typeof data === "object") { |
180 | | - if (!data) { |
181 | | - processNull(data as null); |
182 | | - } else if (Array.isArray(data)) { |
183 | | - processArray(data as object[]); |
184 | | - } else { |
185 | | - processObject(data as object); |
186 | | - } |
187 | | - } else { |
188 | | - processPrimitive(data, typeof data); |
189 | | - } |
190 | | -} |
191 | | -``` |
192 | | - |
193 | | -> **✅ Prefer** |
194 | | -
|
195 | | -Wrap an `any` type as another type for consumers. There are many TypeScript |
196 | | -solutions here, but in general, only at serialization boundaries (postMessage, |
197 | | -HTTP requests) _must_ we transform untyped values. Elsewhere, we should have |
198 | | -validated types. |
199 | | - |
200 | | -```ts |
201 | | -class Data { |
202 | | - private inner: any; |
203 | | - constructor(inner: any) { |
204 | | - this.inner = inner; |
205 | | - } |
206 | | - process() { |
207 | | - // if (typeof this.inner === "object") |
208 | | - } |
209 | | -} |
210 | | - |
211 | | -function processData(data: Data) { |
212 | | - data.process(); |
213 | | -} |
214 | | -``` |
215 | | - |
216 | | -### Avoid representing invalid state |
217 | | - |
218 | | -Similarly, permissive interfaces (including nullable properties and |
219 | | -non-represented exclusive states e.g. "i accept a string or array of strings") |
220 | | -may represent an invalid state at intermediate stages that will need be checked |
221 | | -at every interface: |
222 | | - |
223 | | -> **❌ Avoid** |
224 | | -
|
225 | | -```ts |
226 | | -interface LLMRequest { |
227 | | - prompt?: string; |
228 | | - messages?: string[]; |
229 | | - model?: string; |
230 | | -} |
231 | | - |
232 | | -function request(req: LLMRequest) { |
233 | | - // Not only do we have to modify `req` into a valid |
234 | | - // state here, `processRequest` and any other user of `LLMRequest` |
235 | | - // must also handle this. |
236 | | - |
237 | | - if (!req.model) { |
238 | | - req.model = "default model"; |
239 | | - } |
240 | | - // If both prompt and messages provided, |
241 | | - // use only `messages` |
242 | | - if (req.prompt && req.messages) { |
243 | | - req.prompt = undefined; |
244 | | - } |
245 | | - processRequest(req); |
246 | | -} |
247 | | - |
248 | | -request({ prompt: "hello world" }); |
249 | | -``` |
250 | | - |
251 | | -> **✅ Prefer** |
252 | | -
|
253 | | -For interfaces/types, not allowing unrepresented exclusive states (the prompt |
254 | | -input is always an array; `model` is always defined) requires more explicit |
255 | | -inputs, but then `LLMRequest` is always complete and valid. **Making invalid |
256 | | -states unrepresentable is good**. |
257 | | - |
258 | | -Constructing the request could be also be a class, if we always wanted to apply |
259 | | -appropriate e.g. defaults. |
260 | | - |
261 | | -```ts |
262 | | -enum Model { |
263 | | - Default = "default model"; |
264 | | -} |
265 | | - |
266 | | -interface LLMRequest { |
267 | | - messages: string[], |
268 | | - model: Model, |
269 | | -} |
270 | | - |
271 | | -function request(req: LLMRequest) { |
272 | | - // This is already a valid LLMRequest |
273 | | - processRequest(req); |
274 | | -} |
275 | | - |
276 | | -request({ messages: ["hello world"], model: inputModel ?? Model.Default }); |
277 | | -``` |
278 | | - |
279 | | -### Appropriate Error Handling |
280 | | - |
281 | | -If a function may throw, it's reasonable to wrap it in a try/catch. However, in |
282 | | -complex codebases, handling every error is both tedious and limiting, and may be |
283 | | -preferable to handle errors in a single place with context. Most importantly, |
284 | | -throwing errors is OK, and preventing execution of invalid states is desirable. |
285 | | - |
286 | | -Whether or not an error should be handled in a subprocess could be determined by |
287 | | -whether its a "fatal error" or not: was an assumption invalidated? are we |
288 | | -missing some required capability? Throw an error. Can we continue safely |
289 | | -processing and need to take no further action? Maybe a low-level try/catch is |
290 | | -appropriate. LLMs generally don't have this context and are liberal in their |
291 | | -try/catch usage. Avoid this. |
292 | | - |
293 | | -> **❌ Avoid** |
294 | | -
|
295 | | -In this scenario, errors are logged different ways; if `fetch` throws, we have a |
296 | | -console error log. If `getData()` returns `undefined`, something unexpected |
297 | | -occurred, and there's nothing to be done. `run` should be considered errored and |
298 | | -failed. |
299 | | - |
300 | | -```ts |
301 | | -async function getData(): Promise<string | undefined> { |
302 | | - try { |
303 | | - const res = await fetch(URL); |
304 | | - if (res.ok) { |
305 | | - return res.text(); |
306 | | - } |
307 | | - throw new Error("Unsuccessful HTTP response"); |
308 | | - } catch(e) { |
309 | | - console.error(e); |
310 | | - } |
311 | | -} |
312 | | - |
313 | | -async function run() { |
314 | | - try { |
315 | | - const data = await getData(); |
316 | | - if (data) { |
317 | | - // .. |
318 | | - } |
319 | | - } catch (e) { |
320 | | - console.error("There was an error": e); |
321 | | - } |
322 | | -} |
323 | | -``` |
324 | | - |
325 | | -> **✅ Prefer** |
326 | | -
|
327 | | -In this case, we expect `getData()` to throw, or always return a `string`. Less |
328 | | -handling here, and let the caller determine what to do on failure. |
329 | | - |
330 | | -```ts |
331 | | -async function getData(): Promise<string> { |
332 | | - const res = await fetch(URL); |
333 | | - if (res.ok) { |
334 | | - return res.text(); |
335 | | - } |
336 | | - throw new Error("Unsuccessful HTTP response"); |
337 | | -} |
338 | | - |
339 | | -async function run() { |
340 | | - const data = await getData(); |
341 | | - await processStr(data); |
342 | | -} |
343 | | - |
344 | | -async function main() { |
345 | | - try { |
346 | | - await run(); |
347 | | - } catch (e) { |
348 | | - console.error(e); |
349 | | - } |
350 | | -} |
351 | | -``` |
352 | | - |
353 | | -Sometimes a low-level try/catch is appropriate, of course: |
354 | | - |
355 | | -- `getData()` could have its own try/catch to e.g. retry on failure, throwing |
356 | | - after 3 failed attempts. |
357 | | -- Exposing a `isFeatureSupported(): boolean` function that based on if some |
358 | | - other function throws, determines if "feature" is supported. If we can handle |
359 | | - both scenarios and translate the error into a boolean (e.g. are all of the |
360 | | - ED25519 features we need supported natively for this platform? if not use a |
361 | | - polyfill), then this is not a fatal error, and we explicitly do not want to |
362 | | - throw and handle it elsewhere. |
363 | | - |
364 | | -## Ralph Container Information |
365 | | - |
366 | | -If you are running inside the Ralph Docker container (user is "ralph" or |
367 | | -`/app/start-servers.sh` exists): |
368 | | - |
369 | | -- See `tools/ralph/DEPLOY.md` for Playwright MCP testing and server restart |
370 | | - instructions |
371 | | -- toolshed should run on 8000 and shell on port 5173 |
| 23 | +- `docs/common/RUNTIME.md` - Running servers, testing, and runtime package overview |
| 24 | +- `docs/common/DEVELOPMENT.md` - Coding style, design principles, and best practices |
0 commit comments