From d7d76d4e8df369a1c09d5cb2531edfdeee1352bf Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Mon, 20 Apr 2026 16:23:30 -0400 Subject: [PATCH 01/10] docs(agents): add KO/ViewComponent guidelines from MinuteBox review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port architectural rules from NetPleadings/MinuteBox#9518 (CodeRabbit review config) back into TKO's public LLM-facing docs. Terminology normalized to TKO primitives (LifeCycle/BindingHandler/ComponentABC, ko.subscribable) — no MinuteBox-specific references. - llms.txt: 3 new Gotchas bullets - flat JSX attrs vs `params={{…}}` wrapper - no observable/computed/subscribe inside a computed evaluator - `|| undefined` for binary HTML attrs - agents/contract.md: 3 new sections - "DOM Mutation Containment" — DOM-API calls belong in BindingHandler - "Component Design" — narrow BH scope, component-as-renderer-only - "Component Communication via `subscribable`" — pub/sub over observable-of-function for imperative commands - agents/guide.md: 4 new subsections + 3 new Gotchas bullets - Observables vs computed dependency management - Computed styling via attributes (data-attr + CSS over computed class) - JSX scope rule - Component params in JSX - null (not false) for empty HTML computeds, `|| undefined` for binary attrs, named-variable computeds over inline - agents/verified-behaviors/ - computed.md: new "Anti-patterns" section w/ observable-in-computed leak + test sketch - lifecycle.md: "Usage guidance" for this.computed/this.subscribe - utils-jsx.md: binary HTML attr omit-on-falsy behavior - debug.md: generated stub (no-tests-found) matching package-docs template Co-Authored-By: Claude Opus 4.7 (1M context) --- tko.io/public/agents/contract.md | 74 +++++++++++++++++++ tko.io/public/agents/guide.md | 67 +++++++++++++++++ .../agents/verified-behaviors/computed.md | 19 +++++ .../public/agents/verified-behaviors/debug.md | 19 +++++ .../agents/verified-behaviors/lifecycle.md | 6 ++ .../agents/verified-behaviors/utils-jsx.md | 2 + tko.io/public/llms.txt | 3 + 7 files changed, 190 insertions(+) create mode 100644 tko.io/public/agents/verified-behaviors/debug.md diff --git a/tko.io/public/agents/contract.md b/tko.io/public/agents/contract.md index 9e88e8a9..69a47359 100644 --- a/tko.io/public/agents/contract.md +++ b/tko.io/public/agents/contract.md @@ -40,6 +40,80 @@ In those cases: - let the custom binding read observables and update the DOM - avoid making the custom binding the authoritative owner of app state +## DOM Mutation Containment + +DOM mutation and direct DOM-API calls belong inside a `BindingHandler` (a class that extends `LifeCycle` and is registered as a binding). They do not belong in component view models, utility modules, or arbitrary class methods. + +Violations (do not do outside a `BindingHandler`): +- `document.createElement`, `document.body.appendChild` — bypasses the binding pipeline +- `element.querySelector` / `querySelectorAll` — except to find a mount root for `applyBindings` +- `element.style.*`, `element.classList.*` — use the `style` / `css` bindings +- `element.addEventListener` — use `click`, `event:{…}`, or a binding handler +- `appendChild` / `insertBefore` / `replaceWith` — use `foreach`, `if`, `template` +- `element.innerHTML = …` — use `text` or (when trusted markup is the point) `html`; also flag XSS if content is external +- `element.focus()` / `element.select()` — use `hasFocus` or a focus-orchestration binding +- `requestAnimationFrame` for DOM scheduling — move inside a binding that owns the frame loop +- Storing `HTMLElement` references as component view-model fields — the element belongs to the binding + +Where these calls are safe: +- inside a class that extends `BindingHandler` (its `constructor`/`init`/`update` receive the owning element) +- reading observables whose values drive DOM updates via bindings — that is the whole point +- direct DOM reads in tests/verification code after bindings are active + +Binding handlers are the prescribed escape hatch for imperative DOM work: they receive the exact element, participate in the lifecycle, and dispose cleanly. Mutating the DOM from elsewhere creates a second source of truth for DOM state and the reactive graph loses sight of it. + +## Component Design: Binding Handlers and Component View Models + +### Binding handlers — narrow scope, one DOM task + +Each `BindingHandler` subclass should do one thing to its element. A handler that sanitizes HTML *and* walks the DOM for citations *and* renders diagrams *and* rewrites tables should be split into separate handlers, each composable on the same element. + +Violations: +- a binding handler performing multiple unrelated DOM operations — split by concern +- using a binding handler where a component with JSX would suffice — prefer declarative JSX when the rendering can be expressed that way + +Not a violation: +- a handler that is complex because its single DOM task is inherently complex (rich-text editor, canvas renderer, third-party widget bridge) + +### Component view models — rendering only + +A component view model (a class registered with `components.register(...)`, typically extending `ComponentABC`) should contain only code that produces its template output. Data transformation, parsing, business rules, and utilities belong in standalone `.ts` files the component imports. + +Violations: +- template method exceeding ~80 lines of JSX without decomposition — extract nested child components +- manipulating the DOM directly from a component method — delegate to a binding handler +- non-rendering logic (parsing, domain rules, formatting of structured data) embedded in the component — move to utilities +- instantiating a child component with `params={{ ... }}` wrapping every prop (see "Component Params in JSX" in `/agents/guide.md`) + +Not a violation: +- a component that is large because it composes many small child components +- simple inline computeds that only map data for the template (`const label = ko.pureComputed(() => format(value()))`) + +## Component Communication via `subscribable` + +When a component needs to trigger an imperative action inside a binding handler (for example "print this iframe", "scroll to top", "focus on demand"), pass a `ko.subscribable` owned by the component into the handler. The handler subscribes in its constructor and disposes the subscription in `dispose`. + +Do not model the command as an `observable` holding a function (`observable<(() => void) | null>(null)`). Function-valued observables capture closures that may retain DOM references, muddle cleanup, and do not broadcast. + +```js +// Component owns the channel and fires events +class PrintableCard extends ComponentABC { + printChannel = new ko.subscribable() + onPrintClick = () => this.printChannel.notifySubscribers(null, 'print') +} + +// Binding handler subscribes and disposes with its lifecycle +class PrintIframe extends BindingHandler { + constructor (params) { + super(params) + const channel = this.value + this.addDisposable(channel.subscribe(() => this.$element.contentWindow.print(), null, 'print')) + } +} +``` + +The element reference stays inside the binding handler, `dispose` cleans up naturally, and multiple elements can listen on the same channel without coordination. + ## Security Preference - Prefer `text` over `html`. diff --git a/tko.io/public/agents/guide.md b/tko.io/public/agents/guide.md index 3346dcbb..3912d06e 100644 --- a/tko.io/public/agents/guide.md +++ b/tko.io/public/agents/guide.md @@ -33,6 +33,13 @@ ko.when(viewModel.isReady, () => console.log('Ready')) ko.when(() => viewModel.isReady() && viewModel.hasData()).then(() => console.log('Ready')) ``` +### Observables vs computed dependency management + +- Bare `ko.observable()` and `ko.observableArray()` are safe anywhere — they are containers and do not re-evaluate. +- Inside a class using the `LifeCycle` mixin (for example `BindingHandler`, `ComponentABC`, or anything produced by `LifeCycle.mixInto`), prefer `this.computed(...)` over standalone `ko.computed(...)` and `this.subscribe(observable, cb)` over `observable.subscribe(cb)` — both are auto-disposed when the instance is torn down. +- In plain view models without `LifeCycle`, `ko.computed(...)` and `observable.subscribe(...)` are fine; call `dispose()` on them when you are done. +- **Never** create `ko.observable()`, `ko.computed()`, or call `.subscribe()` inside a computed's evaluator. The evaluator runs on every dependency change, so new observables/subscriptions pile up and never dispose — memory and CPU grow unbounded. Create them once outside the computed, or in a `LifeCycle`-enabled constructor. + ## Bindings Activate with `ko.applyBindings(viewModel, element)`. @@ -65,6 +72,63 @@ When the goal is to demonstrate TKO itself, keep the state flow inside observabl - If an example contrasts reactive models, the counters and highlighted state should also be observable-driven so the example demonstrates the pattern instead of bypassing it. - The line to avoid is using the DOM itself as the mutable source of truth after bindings are active. +### Computed styling via attributes + +Do not use computeds to switch CSS class names or inline style strings. Drive a computed attribute (typically `data-*`) and let CSS select on it. + +```tsx +// Bad: computed className +const className = this.computed(() => this.isActive() ? classes.active : classes.inactive) +return
...
+ +// Bad: computed inline style +const style = this.computed(() => this.isExpanded() ? 'display: block' : 'display: none') +return
...
+ +// Good: computed attribute + CSS selector +const isActive = this.computed(() => this.isActive() || undefined) +return
...
+``` + +```css +.my-component { opacity: 0.5; } +.my-component[data-active] { opacity: 1; } +``` + +This keeps styling logic in CSS, avoids class-name string juggling inside computeds, and pairs naturally with the `|| undefined` pattern for binary attributes (see Gotchas). For classic `data-bind`, the equivalent writer is the `attr` binding: `data-bind="attr: { 'data-active': isActive }"`. + +### JSX scope rule + +Use JSX inside component view models (classes registered with `components.register(...)`, typically extending `ComponentABC`) or in functions whose output is immediately passed to `tko.jsx.render(...)` and mounted. Do not leave JSX in standalone utility functions that outlive a component lifecycle — without `LifeCycle`, subscriptions and computeds created for that JSX have no owner to dispose them. + +```tsx +// Good: component owns the JSX and its lifecycle +class MyCard extends ComponentABC { + template = () =>
+} + +// Also fine: top-level render for a fixed mount +const { node } = tko.jsx.render(
) +document.getElementById('root').appendChild(node) + +// Avoid: JSX returned from a utility module without a clear owner +export const myView = () =>
+``` + +### Component params in JSX + +When instantiating a component from JSX, pass each constructor parameter as its own attribute. Do **not** wrap them in a single `params={{ ... }}` attribute — JSX collects attributes into one object and hands that object to the component constructor, so wrapping in `params` delivers `{ params: { … } }` instead of the flat shape the constructor expects. + +```tsx +// Bad — wrapped in params + + +// Good — each prop is its own attribute + +``` + +The constructor receives `{ output, onView, onEdit }` directly. + ## Classic data-bind parsing and CSP Classic `data-bind` parsing is provider-driven. Use `DataBindProvider` when you need binding strings, and combine it with other providers through `MultiProvider` as needed. @@ -222,6 +286,9 @@ tko.applyBindings({ removeTodo: t => todos.remove(t) }, root) - **Observable children in JSX** are reactive — when an observable's value changes, the DOM updates. If it becomes `undefined`, a placeholder comment is rendered. - **Cannot apply bindings twice** to the same DOM node — throws error. - **`dispose()` on computed/subscriptions** stops updates. After disposal, reading returns last cached value. +- **HTML-producing computeds** should return `null` for empty output, not `false` or an empty fragment — bindings and JSX observers handle `null` cleanly. +- **Binary HTML attributes** (`disabled`, `readonly`, `hidden`, `required`, `checked`, `selected`) omit the attribute when the value is `null`, `undefined`, or `false`. Use `|| undefined` in computeds to make "no attribute" explicit: `disabled={this.computed(() => shouldBeDisabled() || undefined)}`. Never return the string `"false"` — it keeps the attribute set. +- **Prefer named computed variables over inline computeds in JSX.** A computed created inline in JSX is easier to accidentally recreate on each render than one bound to a class field or `const`. ## Testing diff --git a/tko.io/public/agents/verified-behaviors/computed.md b/tko.io/public/agents/verified-behaviors/computed.md index d8dcbe50..ebaaea55 100644 --- a/tko.io/public/agents/verified-behaviors/computed.md +++ b/tko.io/public/agents/verified-behaviors/computed.md @@ -30,3 +30,22 @@ Read this when you need test-backed behavior for `@tko/computed`, especially `co - `when(predicate, callback)` runs the callback once, then disposes its subscription. Notes: The predicate may be either a function or an observable. The return value exposes `dispose()` to cancel the pending notification. With deferred updates enabled, the callback runs in a later task. Specs: `packages/computed/spec/observableUtilsBehaviors.ts`, `packages/computed/spec/asyncBehaviors.ts` + +## Anti-patterns + +- Creating observables, computeds, or subscriptions **inside** a computed's evaluator leaks instances. The evaluator re-runs on every dependency change, producing a new un-disposed subscriber each time, so memory and subscriber count grow without bound. + Test sketch: + ```ts + const dep = ko.observable(0) + const instances: KnockoutObservable[] = [] + const leaky = ko.computed(() => { + const obs = ko.observable(dep() + 1) + instances.push(obs) + return obs() + }) + expect(instances.length).toBe(1) + dep(1); expect(instances.length).toBe(2) + dep(2); expect(instances.length).toBe(3) + ``` + Fix: create the observable/subscription once outside the computed, or inside a `LifeCycle` subclass constructor where `this.subscribe` / `this.computed` own disposal. + Related specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` (disposal semantics). diff --git a/tko.io/public/agents/verified-behaviors/debug.md b/tko.io/public/agents/verified-behaviors/debug.md new file mode 100644 index 00000000..8353c5e4 --- /dev/null +++ b/tko.io/public/agents/verified-behaviors/debug.md @@ -0,0 +1,19 @@ +# Verified Behaviors: @tko/debug + +> Generated from package discovery plus package-local curated unit-test-backed JSON. +> If a behavior is not covered by unit tests, it does not belong in this directory. + +Verified behaviors for @tko/debug. + +## When to Read This + +Read this when you need test-backed behavior for `@tko/debug`, especially verified behaviors for @tko/debug. + +## Status + +- Status: no-tests-found +- Summary: No tests found for this package. + +## Next Step + +Add tests first, then publish curated verified behaviors once the behavior contract is covered. diff --git a/tko.io/public/agents/verified-behaviors/lifecycle.md b/tko.io/public/agents/verified-behaviors/lifecycle.md index 3f287559..e6c4122f 100644 --- a/tko.io/public/agents/verified-behaviors/lifecycle.md +++ b/tko.io/public/agents/verified-behaviors/lifecycle.md @@ -28,3 +28,9 @@ Read this when you need test-backed behavior for `@tko/lifecycle`, especially li Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` - Anchoring one lifecycle object to another with `anchorTo(...)` causes disposal of the parent lifecycle to dispose the anchored child as well. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` + +## Usage guidance + +- Prefer `this.computed(...)` over standalone `ko.computed(...)` inside a `LifeCycle` subclass — the computed is added to the instance's disposal set automatically. +- Prefer `this.subscribe(observable, callback)` over `observable.subscribe(callback)` inside a `LifeCycle` subclass for the same reason. +- Do not create observables, computeds, or subscriptions inside a computed's evaluator — see the anti-pattern in `computed.md`. diff --git a/tko.io/public/agents/verified-behaviors/utils-jsx.md b/tko.io/public/agents/verified-behaviors/utils-jsx.md index 10572495..cf595f74 100644 --- a/tko.io/public/agents/verified-behaviors/utils-jsx.md +++ b/tko.io/public/agents/verified-behaviors/utils-jsx.md @@ -30,3 +30,5 @@ Read this when you need test-backed behavior for `@tko/utils.jsx`, especially JS Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` - JSX-created nodes can carry native bindings and participate in component rendering and observable-array diff updates. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` +- Binary HTML attributes (`disabled`, `readonly`, `hidden`, `required`, `checked`, `selected`) omit the attribute when the observable/computed value is `null`, `undefined`, or `false`; any other value (including the string `"false"`) sets the attribute. Use `|| undefined` in computeds to express "no attribute" explicitly. + Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` diff --git a/tko.io/public/llms.txt b/tko.io/public/llms.txt index b6d1ce88..a566581e 100644 --- a/tko.io/public/llms.txt +++ b/tko.io/public/llms.txt @@ -46,6 +46,9 @@ - Derived `ko-*` values must stay observable/computed — inline expressions freeze - `ko.applyBindings(...)` returns a Promise - Inside `ko-foreach`, binding-context vars use strings: `ko-text="$data"` (not `{$data}`) +- Component JSX params: pass each constructor param as its own attribute (``), never wrap in `params={{...}}` — JSX attrs flatten into one constructor arg, so wrapping double-nests to `{ params: {...} }` +- Never create `ko.observable()`, `ko.computed()`, or `.subscribe()` inside a computed evaluator — re-runs leak instances +- Binary HTML attrs (`disabled`, `hidden`, …): return `|| undefined` from computed; `false` renders the string `"false"` and keeps the attribute ## Browser JSX (esbuild-wasm) From ea158029358e98074bcdbc038a8f658392eeaccc Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Mon, 20 Apr 2026 16:27:48 -0400 Subject: [PATCH 02/10] =?UTF-8?q?Remove=20orphan=20debug.md=20=E2=80=94=20?= =?UTF-8?q?@tko/debug=20not=20on=20main?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stub was locally generated during an earlier run while the codex/tko-mcp-debug-plan branch (which introduces packages/debug) was in the working tree. That package is not merged to main, so the doc referenced a package that does not exist in this branch's scope. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../public/agents/verified-behaviors/debug.md | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 tko.io/public/agents/verified-behaviors/debug.md diff --git a/tko.io/public/agents/verified-behaviors/debug.md b/tko.io/public/agents/verified-behaviors/debug.md deleted file mode 100644 index 8353c5e4..00000000 --- a/tko.io/public/agents/verified-behaviors/debug.md +++ /dev/null @@ -1,19 +0,0 @@ -# Verified Behaviors: @tko/debug - -> Generated from package discovery plus package-local curated unit-test-backed JSON. -> If a behavior is not covered by unit tests, it does not belong in this directory. - -Verified behaviors for @tko/debug. - -## When to Read This - -Read this when you need test-backed behavior for `@tko/debug`, especially verified behaviors for @tko/debug. - -## Status - -- Status: no-tests-found -- Summary: No tests found for this package. - -## Next Step - -Add tests first, then publish curated verified behaviors once the behavior contract is covered. From 5536845db20fd14a796e00ee0d4276b5f34f9d88 Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Mon, 20 Apr 2026 16:29:05 -0400 Subject: [PATCH 03/10] AGENTS.md: verify every doc reference exists on target branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preventive rule added after shipping an orphan verified-behaviors stub (`debug.md`) in the preceding commit — the file referenced `@tko/debug`, a package that exists only on an unmerged branch (`codex/tko-mcp-debug-plan`). Stub had leaked into the working tree from a prior generator run. - New subsection under "Agent-First Documentation": mandatory verification checklist (git ls-files, package existence, spec paths, URLs) before staging any doc file. - Added as a failure mode under "Review Your Own Change Adversarially". Rationale: shipping docs that name non-existent entities is worse than no doc at all — it misleads every future reader (human or agent) who trusts the docs as a contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 98418830..185f930c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -176,6 +176,27 @@ changes — update **both** the Starlight docs (for humans) and the agent guide (for agents). The agent guide should be token-efficient: dense, code-first, minimal prose. +### Never ship docs that reference things that don't exist on the target branch + +Before including a doc file — especially any untracked/generated file you find +in the working tree — verify every package, export, class, function, spec path, +or URL it names actually exists on the branch you are merging into (normally +`main`). + +Mandatory checks before staging a docs file: + +1. `git ls-files ` — is it tracked? If not, where did it come from? + - If it was emitted by a generator (e.g. `tko.io/scripts/generate-verified-behaviors.mjs`), re-run the generator on a clean checkout of the target branch and diff. If the generator does not produce it, it is stale — do not ship it. + - If it was hand-written on a different branch, confirm that branch has merged into the target. `git log --all -- ` and `git branch --contains ` will show where it lives. +2. For each package name in the doc: `ls packages/` and confirm a matching `package.json`. Orphan `@tko/*` references mislead both humans and downstream agents. +3. For each spec path in the doc: the file must exist at the named location. +4. For each external URL in the doc: it is OK to trust user-provided URLs, but do not invent them. + +The failure mode is shipping a doc that promises behaviour the code does not +deliver. That is worse than no doc at all — it poisons every future reader +(human or agent) that trusts the docs as a contract. When in doubt, leave it +out and open a follow-up. + ## Docs Verification When validating `tko.io` documentation changes with the local docs site: @@ -217,6 +238,7 @@ Failure modes specific to a published low-level framework, worth probing every t - Patching the symptom, not the root cause - Unrelated refactors or opportunistic redesigns that balloon the PR (the "Always Improve" bar is *small, low-risk, in-scope* fixes — anything larger belongs in its own PR) - Silent assumptions about environment, timing, or ordering +- Docs that reference packages, APIs, or spec paths that do not exist on the target branch (see "Agent-First Documentation" → "Never ship docs that reference things that don't exist on the target branch") If the change doesn't survive a ten-minute attempt to poke holes in it, it's not ready. From 9906ffba5b9dc424375fea6c6fa4924cb078a983 Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Mon, 20 Apr 2026 16:36:28 -0400 Subject: [PATCH 04/10] AGENTS.md: mandate adversarial review on every in-scope change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promotes the previously-soft "independent second pass" guidance into a hard requirement, scoped to changes that affect package behavior, public API, tests, agent-facing docs, CI, or build/ config. Proportionality carve-out excludes typos, whitespace, and the adversarial review's own report (terminates recursion). How-to covers: fresh subagent, brief with artefact + claim only (withhold author reasoning to prevent anchoring), enumerate failure modes before checking correctness, verify findings defensively (not as licence to override), route out-of-scope findings to follow-ups rather than ballooning the PR, and record the pass outcome in the PR description so compliance is auditable. Written in response to shipping an orphan doc stub in commit d7d76d4e — single-pair-of-eyes review missed it. Rule itself reviewed adversarially before landing; the six findings from that pass (self-reference termination, single-pair leak on verification, proportionality, gaming via leading questions, missing audit trail, PR-scope conflict) are all folded into this wording. Adversarial pass: Explore subagent. Result: flagged 6; all addressed in this commit. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 185f930c..c4446944 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -223,7 +223,35 @@ Avoid scope creep. If an improvement would balloon the PR, file a follow-up issu ## Review Your Own Change Adversarially -Before declaring a change done, steelman the case against it. Ask what could go wrong, what assumption could be false, what future goal it quietly forecloses, what coverage or signal it weakens, who it surprises. Get an independent second pass (a colleague, a subagent where available) on changes that touch framework internals, test coverage, public APIs, or docs the whole team relies on. +Before declaring a change done, steelman the case against it. Ask what could go wrong, what assumption could be false, what future goal it quietly forecloses, what coverage or signal it weakens, who it surprises. + +**Adversarial review is mandatory, not optional, for anything that changes package behavior, public API, test coverage, docs the team relies on, CI workflows, or build/config that affects the matrix.** A single pair of eyes (yours) is not enough in a dark factory. If small teams plus agents are going to maintain what once took a big team, the missing human reviewer has to be replaced by a second agent that was not told what "good" looks like and is asked only "where is this wrong?". + +In scope (always run the pass): +- Code changes in `packages/*` or `builds/*` +- Test additions, deletions, or env changes +- Public `@tko/*` API surface +- Docs in `tko.io/public/` and agent-facing files (`llms.txt`, `agents/*`) +- CI workflows, `tools/build.ts`, `vitest.config.ts`, `biome.json` +- Changesets and commit messages that land a PR + +Out of scope (proportionality): +- Typos, whitespace, comment corrections that do not change behavior or public surface +- The adversarial review itself — its report is not an artefact that needs its own review. One pass per change closes the loop. + +How to run the pass, every time it is in scope: + +- Spawn a fresh subagent (`Agent` tool, specialized `subagent_type` when one fits, otherwise `Explore`). Do **not** let the agent that produced the change also sign it off — that is a null check. +- Brief the reviewer with the **artefact (diff or file) and the claim it makes** ("this PR does X"). Do **not** include your reasoning for why it works, the commit message you intend to write, or the PR title. Anchoring the reviewer to your conclusion defeats the pass. +- Prompt it to enumerate likely failure modes *before* reviewing for correctness (e.g. "list the three most likely ways this could break, then check each"). Ask: "where is this wrong, what would break, what would mislead a future reader?" Bias toward flagging. +- Apply the failure-modes list below *and* the domain-specific checklist for the artefact type (docs → "Never ship docs that reference things that don't exist"; tests → disposal leaks + env assumptions; public API → backwards-compat + changeset; etc.). +- If the reviewer returns a finding, **verify the specific claim** (re-run the command, read the named file, grep for the named symbol) — do not rely on your own prior reasoning to dismiss it. Subagents produce false positives, so verification is defensive, not a licence to override. If after verification you still believe the finding is wrong, record the reasoning in the PR description or as a code comment so a future reader (or maintainer) can judge it; do not silently reject. +- If the pass surfaces a finding that belongs in a separate PR, file a follow-up or spawn a task rather than expanding the current change — keep "Keep PRs focused" intact. +- Record that the pass was run. A single line in the PR description is enough: + `Adversarial pass: . Result: clean` or + `Adversarial pass: . Flagged : . Resolved: .` + Without an audit trail, compliance is unverifiable and the rule is trivially gamed. +- Only after the pass returns clean (or returns findings that you have verified and addressed, deferred to a follow-up, or consciously rejected with documented reasoning) may you declare the work done. This is the ceiling on "Always Improve": that section pushes toward *more* in a PR; this one pushes toward *scrutiny* of what's in it. Use both — improve in scope, audit the scope itself here. From c9a9f49e78f6034fc34c03683dc0b1694914f2e1 Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Mon, 20 Apr 2026 16:47:07 -0400 Subject: [PATCH 05/10] docs(agents): caveman-compress agent-only docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply the caveman:compress skill to files that are purely LLM-facing (llms.txt + verified-behaviors/*.md). Drops articles, filler, and hedging without touching code, backticked identifiers, URLs, or spec paths. Byte savings per file: - tko.io/public/llms.txt: 1% - tko.io/public/agents/verified-behaviors/computed.md: 7% - tko.io/public/agents/verified-behaviors/lifecycle.md:7% - tko.io/public/agents/verified-behaviors/utils-jsx.md:5% Files already dense, so savings are modest — kept anyway because every downstream agent that loads the agent-facing pack pays the token cost on every session. Dual-audience docs (AGENTS.md, agents/guide.md, agents/contract.md) intentionally left uncompressed — human contributors read them. .gitignore: add `*.original.md` so future compress runs don't leak their local backups into commits. Adversarial pass: Explore subagent. Flagged 1 (computed.md fragment-header dropped the If/then conditional → meaning ambiguous). Resolved: restored "If … , not belong in this directory." phrasing in that one line. Other diffs accepted — meaning preserved under caveman article/pronoun drops. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 1 + .../agents/verified-behaviors/computed.md | 26 +++++++++---------- .../agents/verified-behaviors/lifecycle.md | 22 ++++++++-------- .../agents/verified-behaviors/utils-jsx.md | 22 ++++++++-------- tko.io/public/llms.txt | 16 ++++++------ 5 files changed, 44 insertions(+), 43 deletions(-) diff --git a/.gitignore b/.gitignore index 2e2c2be6..cf688e41 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ desktop.ini perf/* *.orig *.bak +*.original.md .DS_Store npm-debug.log node_modules diff --git a/tko.io/public/agents/verified-behaviors/computed.md b/tko.io/public/agents/verified-behaviors/computed.md index ebaaea55..f17a08d4 100644 --- a/tko.io/public/agents/verified-behaviors/computed.md +++ b/tko.io/public/agents/verified-behaviors/computed.md @@ -1,13 +1,13 @@ # Verified Behaviors: @tko/computed > Generated from package discovery plus package-local curated unit-test-backed JSON. -> If a behavior is not covered by unit tests, it does not belong in this directory. +> If behavior not covered by unit tests, not belong in this directory. -`computed`, `when`, `throttle`, and `rateLimit` behavior covered by the async/unit specs. +`computed`, `when`, `throttle`, and `rateLimit` behavior covered by async/unit specs. ## When to Read This -Read this when you need test-backed behavior for `@tko/computed`, especially `computed`, `when`, `throttle`, and `rateLimit` behavior covered by the async/unit specs. +Read when need test-backed behavior for `@tko/computed`, especially `computed`, `when`, `throttle`, `rateLimit` from async/unit specs. ## Status @@ -18,22 +18,22 @@ Read this when you need test-backed behavior for `@tko/computed`, especially `co ## Behaviors -- `extend({ throttle: timeout })` delays observable change notifications until writes stop, then emits the latest value. +- `extend({ throttle: timeout })` delays observable change notifications until writes stop, then emits latest value. Specs: `packages/computed/spec/asyncBehaviors.ts` -- `extend({ throttle: timeout })` on a computed delays reevaluation and notification until dependencies stop changing. - Notes: The evaluator still runs once synchronously on initial creation. +- `extend({ throttle: timeout })` on computed delays reevaluation and notification until dependencies stop changing. + Notes: Evaluator still runs once synchronously on initial creation. Specs: `packages/computed/spec/asyncBehaviors.ts` -- `extend({ rateLimit: timeout })` delays default change notifications, while `beforeChange` stays immediate and `spectate` sees each write. +- `extend({ rateLimit: timeout })` delays default change notifications; `beforeChange` stays immediate, `spectate` sees each write. Specs: `packages/computed/spec/asyncBehaviors.ts` -- `rateLimit` supports both `notifyAtFixedRate` and `notifyWhenChangesStop`, and later `rateLimit` settings are used for future notifications. +- `rateLimit` supports `notifyAtFixedRate` and `notifyWhenChangesStop`; later `rateLimit` settings used for future notifications. Specs: `packages/computed/spec/asyncBehaviors.ts` -- `when(predicate, callback)` runs the callback once, then disposes its subscription. - Notes: The predicate may be either a function or an observable. The return value exposes `dispose()` to cancel the pending notification. With deferred updates enabled, the callback runs in a later task. +- `when(predicate, callback)` runs callback once, then disposes subscription. + Notes: Predicate = function or observable. Return value exposes `dispose()` to cancel. With deferred updates, callback runs in later task. Specs: `packages/computed/spec/observableUtilsBehaviors.ts`, `packages/computed/spec/asyncBehaviors.ts` ## Anti-patterns -- Creating observables, computeds, or subscriptions **inside** a computed's evaluator leaks instances. The evaluator re-runs on every dependency change, producing a new un-disposed subscriber each time, so memory and subscriber count grow without bound. +- Creating observables, computeds, or subscriptions **inside** computed evaluator leaks instances. Evaluator re-runs each dependency change, producing new un-disposed subscriber each time — memory and subscriber count grow unbounded. Test sketch: ```ts const dep = ko.observable(0) @@ -47,5 +47,5 @@ Read this when you need test-backed behavior for `@tko/computed`, especially `co dep(1); expect(instances.length).toBe(2) dep(2); expect(instances.length).toBe(3) ``` - Fix: create the observable/subscription once outside the computed, or inside a `LifeCycle` subclass constructor where `this.subscribe` / `this.computed` own disposal. - Related specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` (disposal semantics). + Fix: create observable/subscription once outside computed, or inside `LifeCycle` subclass constructor where `this.subscribe` / `this.computed` own disposal. + Related specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` (disposal semantics). \ No newline at end of file diff --git a/tko.io/public/agents/verified-behaviors/lifecycle.md b/tko.io/public/agents/verified-behaviors/lifecycle.md index e6c4122f..0d2e9855 100644 --- a/tko.io/public/agents/verified-behaviors/lifecycle.md +++ b/tko.io/public/agents/verified-behaviors/lifecycle.md @@ -1,13 +1,13 @@ # Verified Behaviors: @tko/lifecycle > Generated from package discovery plus package-local curated unit-test-backed JSON. -> If a behavior is not covered by unit tests, it does not belong in this directory. +> If behavior not covered by unit tests, not belong in this directory. -Lifecycle mixins for subscriptions, computeds, DOM listeners, and anchored disposal. +Lifecycle mixins for subscriptions, computeds, DOM listeners, anchored disposal. ## When to Read This -Read this when you need test-backed behavior for `@tko/lifecycle`, especially lifecycle mixins for subscriptions, computeds, DOM listeners, and anchored disposal. +Read when need test-backed behavior for `@tko/lifecycle`, especially lifecycle mixins for subscriptions, computeds, DOM listeners, anchored disposal. ## Status @@ -18,19 +18,19 @@ Read this when you need test-backed behavior for `@tko/lifecycle`, especially li ## Behaviors -- `LifeCycle.mixInto(...)` adds `subscribe`, `computed`, `addEventListener`, `anchorTo`, `dispose`, and `addDisposable` to function prototypes, constructed instances, classes, and class instances. +- `LifeCycle.mixInto(...)` adds `subscribe`, `computed`, `addEventListener`, `anchorTo`, `dispose`, `addDisposable` to function prototypes, constructed instances, classes, class instances. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` -- `LifeCycle.computed(...)` accepts method names, instance methods, bound functions, plain functions, arrow functions, and `{ read }` objects. +- `LifeCycle.computed(...)` accepts method names, instance methods, bound functions, plain functions, arrow functions, `{ read }` objects. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` -- `dispose()` tears down subscriptions created through `subscribe(...)` and computeds created through `computed(...)`. +- `dispose()` tears down subscriptions from `subscribe(...)` and computeds from `computed(...)`. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` -- Event listeners added through `addEventListener(...)` are removed on `dispose()`. +- Event listeners added via `addEventListener(...)` removed on `dispose()`. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` -- Anchoring one lifecycle object to another with `anchorTo(...)` causes disposal of the parent lifecycle to dispose the anchored child as well. +- Anchoring lifecycle object to another with `anchorTo(...)` — parent disposal disposes anchored child. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` ## Usage guidance -- Prefer `this.computed(...)` over standalone `ko.computed(...)` inside a `LifeCycle` subclass — the computed is added to the instance's disposal set automatically. -- Prefer `this.subscribe(observable, callback)` over `observable.subscribe(callback)` inside a `LifeCycle` subclass for the same reason. -- Do not create observables, computeds, or subscriptions inside a computed's evaluator — see the anti-pattern in `computed.md`. +- Prefer `this.computed(...)` over standalone `ko.computed(...)` inside `LifeCycle` subclass — computed auto-added to instance disposal set. +- Prefer `this.subscribe(observable, callback)` over `observable.subscribe(callback)` inside `LifeCycle` subclass, same reason. +- Don't create observables, computeds, subscriptions inside computed's evaluator — see anti-pattern in `computed.md`. \ No newline at end of file diff --git a/tko.io/public/agents/verified-behaviors/utils-jsx.md b/tko.io/public/agents/verified-behaviors/utils-jsx.md index cf595f74..b956c717 100644 --- a/tko.io/public/agents/verified-behaviors/utils-jsx.md +++ b/tko.io/public/agents/verified-behaviors/utils-jsx.md @@ -1,13 +1,13 @@ # Verified Behaviors: @tko/utils.jsx > Generated from package discovery plus package-local curated unit-test-backed JSON. -> If a behavior is not covered by unit tests, it does not belong in this directory. +> If behavior not covered by unit tests, not belong in this directory. -JSX object-to-DOM conversion, reactive children and attributes, and async child handling. +JSX object-to-DOM conversion, reactive children and attributes, async child handling. ## When to Read This -Read this when you need test-backed behavior for `@tko/utils.jsx`, especially JSX object-to-DOM conversion, reactive children and attributes, and async child handling. +Read when need test-backed behavior for `@tko/utils.jsx`, especially JSX object-to-DOM conversion, reactive children/attributes, async child handling. ## Status @@ -18,17 +18,17 @@ Read this when you need test-backed behavior for `@tko/utils.jsx`, especially JS ## Behaviors -- JSX objects convert to DOM nodes, including nested children, arrays, generators, sparse arrays, primitives, SVG nodes, and actual DOM nodes. +- JSX objects convert to DOM nodes, including nested children, arrays, generators, sparse arrays, primitives, SVG nodes, actual DOM nodes. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Observable children and attributes stay live: text, nodes, arrays, numbers, attributes, and nested observable structures update in place. +- Observable children and attributes stay live: text, nodes, arrays, numbers, attributes, nested observable structures update in place. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Binding-handler attributes like `ko-x` keep observable references intact instead of being unwrapped at JSX conversion time. +- Binding-handler attributes like `ko-x` keep observable refs intact, not unwrapped at JSX conversion time. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Re-inserting original JSX nodes is supported; re-inserting non-JSX nodes that already carry bindings throws rather than applying bindings twice. +- Re-inserting original JSX nodes supported; re-inserting non-JSX nodes with existing bindings throws instead of applying bindings twice. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Promise and thenable children are supported, including resolved-node binding, removal before or after resolution, and error-to-comment behavior. +- Promise and thenable children supported: resolved-node binding, removal before/after resolution, error-to-comment behavior. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- JSX-created nodes can carry native bindings and participate in component rendering and observable-array diff updates. - Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Binary HTML attributes (`disabled`, `readonly`, `hidden`, `required`, `checked`, `selected`) omit the attribute when the observable/computed value is `null`, `undefined`, or `false`; any other value (including the string `"false"`) sets the attribute. Use `|| undefined` in computeds to express "no attribute" explicitly. +- JSX-created nodes carry native bindings, participate in component rendering and observable-array diff updates. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` +- Binary HTML attributes (`disabled`, `readonly`, `hidden`, `required`, `checked`, `selected`) omit attribute when observable/computed value is `null`, `undefined`, or `false`; any other value (including string `"false"`) sets attribute. Use `|| undefined` in computeds to express "no attribute" explicitly. + Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` \ No newline at end of file diff --git a/tko.io/public/llms.txt b/tko.io/public/llms.txt index a566581e..d4727dab 100644 --- a/tko.io/public/llms.txt +++ b/tko.io/public/llms.txt @@ -12,12 +12,12 @@ ## Agent Docs -- /agents/soul.md — why Knockout works the way it does (design philosophy) +- /agents/soul.md — why Knockout works (design philosophy) - /agents/guide.md — API reference, gotchas, examples - /agents/glossary.md — domain terms, concepts, packages -- /agents/testing.md — how to run and verify TKO code +- /agents/testing.md — run + verify TKO code - /agents/contract.md — state/binding/DOM architecture -- /agents/options.md — ko.options.* — when to use defineOption vs core Options fields +- /agents/options.md — `ko.options.*` — when to use `defineOption` vs core Options fields - /agents/verified-behaviors/index.md — test-backed behavior contracts - /agents/sample-tsx.html — minimal browser TSX scaffold - /examples/ — interactive self-contained HTML examples @@ -44,11 +44,11 @@ ## Gotchas - Derived `ko-*` values must stay observable/computed — inline expressions freeze -- `ko.applyBindings(...)` returns a Promise +- `ko.applyBindings(...)` returns Promise - Inside `ko-foreach`, binding-context vars use strings: `ko-text="$data"` (not `{$data}`) -- Component JSX params: pass each constructor param as its own attribute (``), never wrap in `params={{...}}` — JSX attrs flatten into one constructor arg, so wrapping double-nests to `{ params: {...} }` -- Never create `ko.observable()`, `ko.computed()`, or `.subscribe()` inside a computed evaluator — re-runs leak instances -- Binary HTML attrs (`disabled`, `hidden`, …): return `|| undefined` from computed; `false` renders the string `"false"` and keeps the attribute +- Component JSX params: pass each constructor param as own attribute (``), never wrap in `params={{...}}` — JSX attrs flatten into one constructor arg, wrapping double-nests to `{ params: {...} }` +- Never create `ko.observable()`, `ko.computed()`, or `.subscribe()` inside computed evaluator — re-runs leak instances +- Binary HTML attrs (`disabled`, `hidden`, …): return `|| undefined` from computed; `false` renders string `"false"`, keeps attribute ## Browser JSX (esbuild-wasm) @@ -64,4 +64,4 @@ const result = await esbuild.transform(tsxCode, { - Docs: /observables/ · /computed/ · /bindings/ · /components/ - Playground: /playground -- GitHub: https://github.com/knockout/tko +- GitHub: https://github.com/knockout/tko \ No newline at end of file From 4adc87d6355a894f2742012baf99188e060a9467 Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Tue, 21 Apr 2026 08:26:24 -0400 Subject: [PATCH 06/10] guide.md: rename shadowing local in data-attr styling example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Good: computed attribute + CSS selector" example had `const isActive = this.computed(() => this.isActive() || undefined)`, where the local variable `isActive` shadows the class method `this.isActive()`. JS scoping makes the inner call resolve to the method, so the example was not actually broken — but a novice reader could reasonably mistake it for a self-referential computed. Renamed the local to `activeAttr` so the two names no longer collide. Adversarial pass: Explore subagent on full PR diff. Flagged 1 high (shadowing confusion in this example). Also flagged two items verified as false positives: "not belong" fragments in caveman-compressed files (grammatically terse but unambiguous), and `tko.jsx.render` (defined on the bundled `tko` global at builds/reference/src/index.ts — already documented in existing guide.md counter/todo examples). Co-Authored-By: Claude Opus 4.7 (1M context) --- tko.io/public/agents/guide.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tko.io/public/agents/guide.md b/tko.io/public/agents/guide.md index 3912d06e..a4a41d25 100644 --- a/tko.io/public/agents/guide.md +++ b/tko.io/public/agents/guide.md @@ -86,8 +86,8 @@ const style = this.computed(() => this.isExpanded() ? 'display: block' : 'displa return
...
// Good: computed attribute + CSS selector -const isActive = this.computed(() => this.isActive() || undefined) -return
...
+const activeAttr = this.computed(() => this.isActive() || undefined) +return
...
``` ```css @@ -95,7 +95,7 @@ return
...
.my-component[data-active] { opacity: 1; } ``` -This keeps styling logic in CSS, avoids class-name string juggling inside computeds, and pairs naturally with the `|| undefined` pattern for binary attributes (see Gotchas). For classic `data-bind`, the equivalent writer is the `attr` binding: `data-bind="attr: { 'data-active': isActive }"`. +This keeps styling logic in CSS, avoids class-name string juggling inside computeds, and pairs naturally with the `|| undefined` pattern for binary attributes (see Gotchas). For classic `data-bind`, the equivalent writer is the `attr` binding: `data-bind="attr: { 'data-active': activeAttr }"`. ### JSX scope rule From 10a08a46832a42d16c0be3c9116f81a78accece8 Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Tue, 21 Apr 2026 08:28:42 -0400 Subject: [PATCH 07/10] =?UTF-8?q?Revert=20verified-behaviors=20edits=20?= =?UTF-8?q?=E2=80=94=20generator-owned,=20not=20hand-editable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tko.io/scripts/generate-verified-behaviors.mjs` rebuilds every `tko.io/public/agents/verified-behaviors/*.md` file from the package-local `verified-behaviors.json` curated sources on every `prebuild` / `predev` run (and therefore every CI build via `deploy-docs.yml`). Hand-edits to those markdown files are transient and would have been wiped the next time anyone ran `bun run dev` or the site was deployed. Reverted to the generator's canonical output on `main`: - tko.io/public/agents/verified-behaviors/computed.md - tko.io/public/agents/verified-behaviors/lifecycle.md - tko.io/public/agents/verified-behaviors/utils-jsx.md That also undoes the caveman compression on those three files — the generator emits natural-English from the JSON, so compression would be re-undone on the first build. Compression on `llms.txt` stands (no generator owns it). Content that was in those reverts now lives only in the hand-authored docs where it belongs per verified-behaviors charter ("If behavior not covered by unit tests, it does not belong in this directory"): - Observable-inside-computed leak: `agents/guide.md` Gotchas (strengthens the existing "Don't create computeds/subscriptions inside computeds" bullet to include observables + subscriptions). - `this.computed` / `this.subscribe` preference: `agents/guide.md` under "Observables vs computed dependency management". - Binary HTML attr `|| undefined`: `agents/guide.md` Gotchas + `llms.txt` Gotchas. (No spec in `packages/utils.jsx/spec` covers the null/undefined/false omission behavior, so it cannot be added as a verified entry — would need a test first.) Adversarial pass: caught by the "Docs Verification" pre-check, not by a subagent — running `bun run dev` would have regenerated the three files and silently dropped 3 commits' worth of edits. Audit trail added here. Follow-up: if someone wants those additions as verified entries, the path is (1) add a test to the owning package's spec, (2) add a behavior entry to that package's `verified-behaviors.json`, (3) rerun the generator. Not this PR. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../agents/verified-behaviors/computed.md | 39 +++++-------------- .../agents/verified-behaviors/lifecycle.md | 22 ++++------- .../agents/verified-behaviors/utils-jsx.md | 20 +++++----- 3 files changed, 27 insertions(+), 54 deletions(-) diff --git a/tko.io/public/agents/verified-behaviors/computed.md b/tko.io/public/agents/verified-behaviors/computed.md index f17a08d4..d8dcbe50 100644 --- a/tko.io/public/agents/verified-behaviors/computed.md +++ b/tko.io/public/agents/verified-behaviors/computed.md @@ -1,13 +1,13 @@ # Verified Behaviors: @tko/computed > Generated from package discovery plus package-local curated unit-test-backed JSON. -> If behavior not covered by unit tests, not belong in this directory. +> If a behavior is not covered by unit tests, it does not belong in this directory. -`computed`, `when`, `throttle`, and `rateLimit` behavior covered by async/unit specs. +`computed`, `when`, `throttle`, and `rateLimit` behavior covered by the async/unit specs. ## When to Read This -Read when need test-backed behavior for `@tko/computed`, especially `computed`, `when`, `throttle`, `rateLimit` from async/unit specs. +Read this when you need test-backed behavior for `@tko/computed`, especially `computed`, `when`, `throttle`, and `rateLimit` behavior covered by the async/unit specs. ## Status @@ -18,34 +18,15 @@ Read when need test-backed behavior for `@tko/computed`, especially `computed`, ## Behaviors -- `extend({ throttle: timeout })` delays observable change notifications until writes stop, then emits latest value. +- `extend({ throttle: timeout })` delays observable change notifications until writes stop, then emits the latest value. Specs: `packages/computed/spec/asyncBehaviors.ts` -- `extend({ throttle: timeout })` on computed delays reevaluation and notification until dependencies stop changing. - Notes: Evaluator still runs once synchronously on initial creation. +- `extend({ throttle: timeout })` on a computed delays reevaluation and notification until dependencies stop changing. + Notes: The evaluator still runs once synchronously on initial creation. Specs: `packages/computed/spec/asyncBehaviors.ts` -- `extend({ rateLimit: timeout })` delays default change notifications; `beforeChange` stays immediate, `spectate` sees each write. +- `extend({ rateLimit: timeout })` delays default change notifications, while `beforeChange` stays immediate and `spectate` sees each write. Specs: `packages/computed/spec/asyncBehaviors.ts` -- `rateLimit` supports `notifyAtFixedRate` and `notifyWhenChangesStop`; later `rateLimit` settings used for future notifications. +- `rateLimit` supports both `notifyAtFixedRate` and `notifyWhenChangesStop`, and later `rateLimit` settings are used for future notifications. Specs: `packages/computed/spec/asyncBehaviors.ts` -- `when(predicate, callback)` runs callback once, then disposes subscription. - Notes: Predicate = function or observable. Return value exposes `dispose()` to cancel. With deferred updates, callback runs in later task. +- `when(predicate, callback)` runs the callback once, then disposes its subscription. + Notes: The predicate may be either a function or an observable. The return value exposes `dispose()` to cancel the pending notification. With deferred updates enabled, the callback runs in a later task. Specs: `packages/computed/spec/observableUtilsBehaviors.ts`, `packages/computed/spec/asyncBehaviors.ts` - -## Anti-patterns - -- Creating observables, computeds, or subscriptions **inside** computed evaluator leaks instances. Evaluator re-runs each dependency change, producing new un-disposed subscriber each time — memory and subscriber count grow unbounded. - Test sketch: - ```ts - const dep = ko.observable(0) - const instances: KnockoutObservable[] = [] - const leaky = ko.computed(() => { - const obs = ko.observable(dep() + 1) - instances.push(obs) - return obs() - }) - expect(instances.length).toBe(1) - dep(1); expect(instances.length).toBe(2) - dep(2); expect(instances.length).toBe(3) - ``` - Fix: create observable/subscription once outside computed, or inside `LifeCycle` subclass constructor where `this.subscribe` / `this.computed` own disposal. - Related specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` (disposal semantics). \ No newline at end of file diff --git a/tko.io/public/agents/verified-behaviors/lifecycle.md b/tko.io/public/agents/verified-behaviors/lifecycle.md index 0d2e9855..3f287559 100644 --- a/tko.io/public/agents/verified-behaviors/lifecycle.md +++ b/tko.io/public/agents/verified-behaviors/lifecycle.md @@ -1,13 +1,13 @@ # Verified Behaviors: @tko/lifecycle > Generated from package discovery plus package-local curated unit-test-backed JSON. -> If behavior not covered by unit tests, not belong in this directory. +> If a behavior is not covered by unit tests, it does not belong in this directory. -Lifecycle mixins for subscriptions, computeds, DOM listeners, anchored disposal. +Lifecycle mixins for subscriptions, computeds, DOM listeners, and anchored disposal. ## When to Read This -Read when need test-backed behavior for `@tko/lifecycle`, especially lifecycle mixins for subscriptions, computeds, DOM listeners, anchored disposal. +Read this when you need test-backed behavior for `@tko/lifecycle`, especially lifecycle mixins for subscriptions, computeds, DOM listeners, and anchored disposal. ## Status @@ -18,19 +18,13 @@ Read when need test-backed behavior for `@tko/lifecycle`, especially lifecycle m ## Behaviors -- `LifeCycle.mixInto(...)` adds `subscribe`, `computed`, `addEventListener`, `anchorTo`, `dispose`, `addDisposable` to function prototypes, constructed instances, classes, class instances. +- `LifeCycle.mixInto(...)` adds `subscribe`, `computed`, `addEventListener`, `anchorTo`, `dispose`, and `addDisposable` to function prototypes, constructed instances, classes, and class instances. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` -- `LifeCycle.computed(...)` accepts method names, instance methods, bound functions, plain functions, arrow functions, `{ read }` objects. +- `LifeCycle.computed(...)` accepts method names, instance methods, bound functions, plain functions, arrow functions, and `{ read }` objects. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` -- `dispose()` tears down subscriptions from `subscribe(...)` and computeds from `computed(...)`. +- `dispose()` tears down subscriptions created through `subscribe(...)` and computeds created through `computed(...)`. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` -- Event listeners added via `addEventListener(...)` removed on `dispose()`. +- Event listeners added through `addEventListener(...)` are removed on `dispose()`. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` -- Anchoring lifecycle object to another with `anchorTo(...)` — parent disposal disposes anchored child. +- Anchoring one lifecycle object to another with `anchorTo(...)` causes disposal of the parent lifecycle to dispose the anchored child as well. Specs: `packages/lifecycle/spec/LifeCycleBehaviors.ts` - -## Usage guidance - -- Prefer `this.computed(...)` over standalone `ko.computed(...)` inside `LifeCycle` subclass — computed auto-added to instance disposal set. -- Prefer `this.subscribe(observable, callback)` over `observable.subscribe(callback)` inside `LifeCycle` subclass, same reason. -- Don't create observables, computeds, subscriptions inside computed's evaluator — see anti-pattern in `computed.md`. \ No newline at end of file diff --git a/tko.io/public/agents/verified-behaviors/utils-jsx.md b/tko.io/public/agents/verified-behaviors/utils-jsx.md index b956c717..10572495 100644 --- a/tko.io/public/agents/verified-behaviors/utils-jsx.md +++ b/tko.io/public/agents/verified-behaviors/utils-jsx.md @@ -1,13 +1,13 @@ # Verified Behaviors: @tko/utils.jsx > Generated from package discovery plus package-local curated unit-test-backed JSON. -> If behavior not covered by unit tests, not belong in this directory. +> If a behavior is not covered by unit tests, it does not belong in this directory. -JSX object-to-DOM conversion, reactive children and attributes, async child handling. +JSX object-to-DOM conversion, reactive children and attributes, and async child handling. ## When to Read This -Read when need test-backed behavior for `@tko/utils.jsx`, especially JSX object-to-DOM conversion, reactive children/attributes, async child handling. +Read this when you need test-backed behavior for `@tko/utils.jsx`, especially JSX object-to-DOM conversion, reactive children and attributes, and async child handling. ## Status @@ -18,17 +18,15 @@ Read when need test-backed behavior for `@tko/utils.jsx`, especially JSX object- ## Behaviors -- JSX objects convert to DOM nodes, including nested children, arrays, generators, sparse arrays, primitives, SVG nodes, actual DOM nodes. +- JSX objects convert to DOM nodes, including nested children, arrays, generators, sparse arrays, primitives, SVG nodes, and actual DOM nodes. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Observable children and attributes stay live: text, nodes, arrays, numbers, attributes, nested observable structures update in place. +- Observable children and attributes stay live: text, nodes, arrays, numbers, attributes, and nested observable structures update in place. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Binding-handler attributes like `ko-x` keep observable refs intact, not unwrapped at JSX conversion time. +- Binding-handler attributes like `ko-x` keep observable references intact instead of being unwrapped at JSX conversion time. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Re-inserting original JSX nodes supported; re-inserting non-JSX nodes with existing bindings throws instead of applying bindings twice. +- Re-inserting original JSX nodes is supported; re-inserting non-JSX nodes that already carry bindings throws rather than applying bindings twice. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Promise and thenable children supported: resolved-node binding, removal before/after resolution, error-to-comment behavior. +- Promise and thenable children are supported, including resolved-node binding, removal before or after resolution, and error-to-comment behavior. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- JSX-created nodes carry native bindings, participate in component rendering and observable-array diff updates. +- JSX-created nodes can carry native bindings and participate in component rendering and observable-array diff updates. Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` -- Binary HTML attributes (`disabled`, `readonly`, `hidden`, `required`, `checked`, `selected`) omit attribute when observable/computed value is `null`, `undefined`, or `false`; any other value (including string `"false"`) sets attribute. Use `|| undefined` in computeds to express "no attribute" explicitly. - Specs: `packages/utils.jsx/spec/jsxBehaviors.ts` \ No newline at end of file From 92999a2d17bf02382b8e147f6c4ce4f758a604cf Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Tue, 21 Apr 2026 08:41:03 -0400 Subject: [PATCH 08/10] Slim AGENTS.md: extract process rules to agents/process.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md is loaded on every agent session — token cost amortized forever. Two recently-added process blocks ("Never ship docs that reference things that don't exist" + "Adversarial review is mandatory") were ~50 lines combined. Moved the how-to detail to a new hand-authored doc `tko.io/public/agents/process.md`; AGENTS.md keeps the one-line mandate, in-scope trigger, and dark-factory motivation inline so the rule still fires from session load. Changes: - `AGENTS.md`: two heavy subsections collapsed to one-paragraph mandates with links to process.md; failure-modes list + existing narrative retained inline. - `tko.io/public/agents/process.md`: new file carrying the full how-to, in-scope/out-of-scope list, adversarial-review checklist, audit-trail format, and generator-ownership warning for verified-behaviors. Includes a credits section naming the MinuteBox PR that seeded the imported architectural rules. - `tko.io/public/llms.txt`: added `/agents/process.md` to the agent-docs list so downstream agents discover it. Line delta vs `main` for AGENTS.md: +5 (down from +55 before the refactor). Credit: architectural review guidelines originally from NetPleadings/MinuteBox#9518 by @jameskozlowskimb with input from @ctcarton — ported upstream and reworded for TKO primitives. Adversarial pass: Explore subagent. Flagged 4 items; 1 valid (lost dark-factory motivation — restored one sentence inline), 3 false positives (link-resolution claim wrong for repo-relative markdown paths that Claude Code follows natively; in-scope list claim wrong because list stayed inline in prose; circular-ref claim wrong because the bidirectional link is content vs. rules, not a cycle). Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 49 +-------------------- tko.io/public/agents/process.md | 75 +++++++++++++++++++++++++++++++++ tko.io/public/llms.txt | 1 + 3 files changed, 78 insertions(+), 47 deletions(-) create mode 100644 tko.io/public/agents/process.md diff --git a/AGENTS.md b/AGENTS.md index c4446944..2273b38f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -176,26 +176,7 @@ changes — update **both** the Starlight docs (for humans) and the agent guide (for agents). The agent guide should be token-efficient: dense, code-first, minimal prose. -### Never ship docs that reference things that don't exist on the target branch - -Before including a doc file — especially any untracked/generated file you find -in the working tree — verify every package, export, class, function, spec path, -or URL it names actually exists on the branch you are merging into (normally -`main`). - -Mandatory checks before staging a docs file: - -1. `git ls-files ` — is it tracked? If not, where did it come from? - - If it was emitted by a generator (e.g. `tko.io/scripts/generate-verified-behaviors.mjs`), re-run the generator on a clean checkout of the target branch and diff. If the generator does not produce it, it is stale — do not ship it. - - If it was hand-written on a different branch, confirm that branch has merged into the target. `git log --all -- ` and `git branch --contains ` will show where it lives. -2. For each package name in the doc: `ls packages/` and confirm a matching `package.json`. Orphan `@tko/*` references mislead both humans and downstream agents. -3. For each spec path in the doc: the file must exist at the named location. -4. For each external URL in the doc: it is OK to trust user-provided URLs, but do not invent them. - -The failure mode is shipping a doc that promises behaviour the code does not -deliver. That is worse than no doc at all — it poisons every future reader -(human or agent) that trusts the docs as a contract. When in doubt, leave it -out and open a follow-up. +**Before staging any doc, verify every package, spec path, and URL it names exists on the target branch.** Pay extra attention to untracked or generated files in the working tree. Full checklist: [`tko.io/public/agents/process.md`](tko.io/public/agents/process.md#never-ship-docs-that-reference-things-that-dont-exist-on-the-target-branch). ## Docs Verification @@ -225,33 +206,7 @@ Avoid scope creep. If an improvement would balloon the PR, file a follow-up issu Before declaring a change done, steelman the case against it. Ask what could go wrong, what assumption could be false, what future goal it quietly forecloses, what coverage or signal it weakens, who it surprises. -**Adversarial review is mandatory, not optional, for anything that changes package behavior, public API, test coverage, docs the team relies on, CI workflows, or build/config that affects the matrix.** A single pair of eyes (yours) is not enough in a dark factory. If small teams plus agents are going to maintain what once took a big team, the missing human reviewer has to be replaced by a second agent that was not told what "good" looks like and is asked only "where is this wrong?". - -In scope (always run the pass): -- Code changes in `packages/*` or `builds/*` -- Test additions, deletions, or env changes -- Public `@tko/*` API surface -- Docs in `tko.io/public/` and agent-facing files (`llms.txt`, `agents/*`) -- CI workflows, `tools/build.ts`, `vitest.config.ts`, `biome.json` -- Changesets and commit messages that land a PR - -Out of scope (proportionality): -- Typos, whitespace, comment corrections that do not change behavior or public surface -- The adversarial review itself — its report is not an artefact that needs its own review. One pass per change closes the loop. - -How to run the pass, every time it is in scope: - -- Spawn a fresh subagent (`Agent` tool, specialized `subagent_type` when one fits, otherwise `Explore`). Do **not** let the agent that produced the change also sign it off — that is a null check. -- Brief the reviewer with the **artefact (diff or file) and the claim it makes** ("this PR does X"). Do **not** include your reasoning for why it works, the commit message you intend to write, or the PR title. Anchoring the reviewer to your conclusion defeats the pass. -- Prompt it to enumerate likely failure modes *before* reviewing for correctness (e.g. "list the three most likely ways this could break, then check each"). Ask: "where is this wrong, what would break, what would mislead a future reader?" Bias toward flagging. -- Apply the failure-modes list below *and* the domain-specific checklist for the artefact type (docs → "Never ship docs that reference things that don't exist"; tests → disposal leaks + env assumptions; public API → backwards-compat + changeset; etc.). -- If the reviewer returns a finding, **verify the specific claim** (re-run the command, read the named file, grep for the named symbol) — do not rely on your own prior reasoning to dismiss it. Subagents produce false positives, so verification is defensive, not a licence to override. If after verification you still believe the finding is wrong, record the reasoning in the PR description or as a code comment so a future reader (or maintainer) can judge it; do not silently reject. -- If the pass surfaces a finding that belongs in a separate PR, file a follow-up or spawn a task rather than expanding the current change — keep "Keep PRs focused" intact. -- Record that the pass was run. A single line in the PR description is enough: - `Adversarial pass: . Result: clean` or - `Adversarial pass: . Flagged : . Resolved: .` - Without an audit trail, compliance is unverifiable and the rule is trivially gamed. -- Only after the pass returns clean (or returns findings that you have verified and addressed, deferred to a follow-up, or consciously rejected with documented reasoning) may you declare the work done. +**Adversarial review is mandatory for in-scope changes** (code, tests, public API, agent-facing docs, CI, `tools/build.ts`, `vitest.config.ts`, `biome.json`, landing commit messages). A single pair of eyes (yours) is not enough in a dark factory — the missing human reviewer has to be replaced by a second agent that was not told what "good" looks like and is asked only "where is this wrong?". Spawn a fresh subagent, brief it with the artefact + claim only (no author reasoning), bias toward flagging, verify any findings defensively, and record the outcome in the PR description. Out of scope: typos, whitespace, comment corrections. Full how-to and audit-trail format: [`tko.io/public/agents/process.md`](tko.io/public/agents/process.md#adversarial-review-is-mandatory). This is the ceiling on "Always Improve": that section pushes toward *more* in a PR; this one pushes toward *scrutiny* of what's in it. Use both — improve in scope, audit the scope itself here. diff --git a/tko.io/public/agents/process.md b/tko.io/public/agents/process.md new file mode 100644 index 00000000..d61986f0 --- /dev/null +++ b/tko.io/public/agents/process.md @@ -0,0 +1,75 @@ +# Agent Process Rules + +Mandatory workflow rules for AI agents working in the TKO repo. AGENTS.md +references this file for the detail of each rule; the one-line mandate lives +there so it fires on session load. Read the matching section here before +declaring work complete. + +## Never ship docs that reference things that don't exist on the target branch + +Before including a doc file — especially any untracked/generated file you find +in the working tree — verify every package, export, class, function, spec path, +or URL it names actually exists on the branch you are merging into (normally +`main`). + +Mandatory checks before staging a docs file: + +1. `git ls-files ` — is it tracked? If not, where did it come from? + - If it was emitted by a generator (e.g. `tko.io/scripts/generate-verified-behaviors.mjs`), re-run the generator on a clean checkout of the target branch and diff. If the generator does not produce it, it is stale — do not ship it. + - If it was hand-written on a different branch, confirm that branch has merged into the target. `git log --all -- ` and `git branch --contains ` will show where it lives. +2. For each package name in the doc: `ls packages/` and confirm a matching `package.json`. Orphan `@tko/*` references mislead both humans and downstream agents. +3. For each spec path in the doc: the file must exist at the named location. +4. For each external URL in the doc: it is OK to trust user-provided URLs, but do not invent them. + +The failure mode is shipping a doc that promises behaviour the code does not +deliver. That is worse than no doc at all — it poisons every future reader +(human or agent) that trusts the docs as a contract. When in doubt, leave it +out and open a follow-up. + +Also note: `tko.io/public/agents/verified-behaviors/*.md` are regenerated from +`packages/*/verified-behaviors.json` on every `prebuild` / `predev` / CI build. +Hand-edits there are transient — the generator wins. Edit the JSON source, or +put hand-authored guidance in `guide.md` / `contract.md` instead. + +## Adversarial review is mandatory + +A single pair of eyes (yours) is not enough in a dark factory. If small teams +plus agents are going to maintain what once took a big team, the missing human +reviewer has to be replaced by a second agent that was not told what "good" +looks like and is asked only "where is this wrong?". + +**In scope** (always run the pass): +- Code changes in `packages/*` or `builds/*` +- Test additions, deletions, or env changes +- Public `@tko/*` API surface +- Docs in `tko.io/public/` and agent-facing files (`llms.txt`, `agents/*`) +- CI workflows, `tools/build.ts`, `vitest.config.ts`, `biome.json` +- Changesets and commit messages that land a PR + +**Out of scope** (proportionality): +- Typos, whitespace, comment corrections that do not change behavior or public surface +- The adversarial review itself — its report is not an artefact that needs its own review. One pass per change closes the loop. + +**How to run the pass**, every time it is in scope: + +- Spawn a fresh subagent (`Agent` tool, specialized `subagent_type` when one fits, otherwise `Explore`). Do **not** let the agent that produced the change also sign it off — that is a null check. +- Brief the reviewer with the **artefact (diff or file) and the claim it makes** ("this PR does X"). Do **not** include your reasoning for why it works, the commit message you intend to write, or the PR title. Anchoring the reviewer to your conclusion defeats the pass. +- Prompt it to enumerate likely failure modes *before* reviewing for correctness (e.g. "list the three most likely ways this could break, then check each"). Ask: "where is this wrong, what would break, what would mislead a future reader?" Bias toward flagging. +- Apply the AGENTS.md failure-modes list *and* the domain-specific checklist for the artefact type (docs → "Never ship docs that reference things that don't exist"; tests → disposal leaks + env assumptions; public API → backwards-compat + changeset; etc.). +- If the reviewer returns a finding, **verify the specific claim** (re-run the command, read the named file, grep for the named symbol) — do not rely on your own prior reasoning to dismiss it. Subagents produce false positives, so verification is defensive, not a licence to override. If after verification you still believe the finding is wrong, record the reasoning in the PR description or as a code comment so a future reader (or maintainer) can judge it; do not silently reject. +- If the pass surfaces a finding that belongs in a separate PR, file a follow-up or spawn a task rather than expanding the current change — keep "Keep PRs focused" intact. +- Record that the pass was run. A single line in the PR description is enough: + `Adversarial pass: . Result: clean` or + `Adversarial pass: . Flagged : . Resolved: .` + Without an audit trail, compliance is unverifiable and the rule is trivially gamed. +- Only after the pass returns clean (or returns findings that you have verified and addressed, deferred to a follow-up, or consciously rejected with documented reasoning) may you declare the work done. + +## Credits + +Architectural review guidelines at `agents/contract.md` ("DOM Mutation +Containment", "Component Design", "Component Communication via +`subscribable`") and related Gotchas in `agents/guide.md` and `llms.txt` +originate from code-review rules in the MinuteBox project +([NetPleadings/MinuteBox#9518](https://github.com/NetPleadings/MinuteBox/pull/9518), +@jameskozlowskimb) with input from @ctcarton, ported upstream and reworded for +TKO primitives. diff --git a/tko.io/public/llms.txt b/tko.io/public/llms.txt index d4727dab..c320f2bf 100644 --- a/tko.io/public/llms.txt +++ b/tko.io/public/llms.txt @@ -18,6 +18,7 @@ - /agents/testing.md — run + verify TKO code - /agents/contract.md — state/binding/DOM architecture - /agents/options.md — `ko.options.*` — when to use `defineOption` vs core Options fields +- /agents/process.md — mandatory agent workflow (doc-ref verification, adversarial review) - /agents/verified-behaviors/index.md — test-backed behavior contracts - /agents/sample-tsx.html — minimal browser TSX scaffold - /examples/ — interactive self-contained HTML examples From 5eeb04f8484a46458f32a19eef36485ee82e031c Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Tue, 21 Apr 2026 08:46:34 -0400 Subject: [PATCH 09/10] guide.md: reframe computed-styling section as preference, not mandate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous wording cast Pattern B (computed data-attr + CSS selector) as the "correct" approach and Pattern A (computed className / inline style) as an anti-pattern. That's a style preference, not a framework rule — both produce the same runtime behaviour; consistency within a codebase matters more than the choice. Rewrote the section to present both patterns symmetrically with trade-offs each way. Key change: "Do not use computeds to switch CSS class names …" becomes "Reactive styling: two valid patterns … Pick one and apply it consistently." Adversarial pass: Explore subagent. Flagged 2 valid items in the trade-offs list — (1) Pattern A's "rule stays in component file" claim incomplete when class body lives in a separate stylesheet; (2) Pattern B's "avoids class-name string concatenation" read as pro-B framing disguised as neutral. Both tweaked to acknowledge Pattern A's template-literal / class-map mitigation and the split-visibility caveat. Result: balanced. Co-Authored-By: Claude Opus 4.7 (1M context) --- tko.io/public/agents/guide.md | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tko.io/public/agents/guide.md b/tko.io/public/agents/guide.md index a4a41d25..28e882dc 100644 --- a/tko.io/public/agents/guide.md +++ b/tko.io/public/agents/guide.md @@ -72,20 +72,20 @@ When the goal is to demonstrate TKO itself, keep the state flow inside observabl - If an example contrasts reactive models, the counters and highlighted state should also be observable-driven so the example demonstrates the pattern instead of bypassing it. - The line to avoid is using the DOM itself as the mutable source of truth after bindings are active. -### Computed styling via attributes +### Reactive styling: two valid patterns -Do not use computeds to switch CSS class names or inline style strings. Drive a computed attribute (typically `data-*`) and let CSS select on it. +Reactive state that drives visual variations can be expressed two ways. Neither is universally "correct" — both produce the same runtime behaviour. **Pick one and apply it consistently across a codebase.** Mixing the two for the same kind of state is what hurts readability. + +**Pattern A — computed class name / inline style.** The computed returns the exact class name or style string. Styling lives next to the reactive expression. ```tsx -// Bad: computed className const className = this.computed(() => this.isActive() ? classes.active : classes.inactive) return
...
+``` -// Bad: computed inline style -const style = this.computed(() => this.isExpanded() ? 'display: block' : 'display: none') -return
...
+**Pattern B — computed attribute, CSS selector does the switching.** The computed returns a flag value; CSS reads the attribute and applies styles. -// Good: computed attribute + CSS selector +```tsx const activeAttr = this.computed(() => this.isActive() || undefined) return
...
``` @@ -95,7 +95,13 @@ return
...
.my-component[data-active] { opacity: 1; } ``` -This keeps styling logic in CSS, avoids class-name string juggling inside computeds, and pairs naturally with the `|| undefined` pattern for binary attributes (see Gotchas). For classic `data-bind`, the equivalent writer is the `attr` binding: `data-bind="attr: { 'data-active': activeAttr }"`. +**Trade-offs.** +- Pattern A keeps the rule ("active → this class") visible in the component file; refactors that rename classes stay local. Split visibility if the class body lives in a separate stylesheet. +- Pattern B keeps styling in CSS and pairs naturally with the `|| undefined` pattern for binary attributes (see Gotchas); adding a new visual variant means editing CSS, not the component. +- Pattern B sidesteps class-name string concatenation inside computeds for complex multi-state styling. Pattern A can avoid the same footgun with template literals or a class-map helper; it just doesn't get it for free. +- Pattern A reads more directly when one class encapsulates many unrelated properties that Pattern B would need multiple attribute selectors to reach. + +Whichever pattern the codebase adopts, the corresponding classic `data-bind` writers are `css:` / `style:` for pattern A, or `attr:` for pattern B (`data-bind="attr: { 'data-active': activeAttr }"`). ### JSX scope rule From 5f9ad7948a4233c10bfa15a0b7816783f2224c60 Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Tue, 21 Apr 2026 08:50:37 -0400 Subject: [PATCH 10/10] Guide + process tweaks per maintainer feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two non-mandate issues surfaced: 1. The "Computed styling via attributes" section was written as if the two shown patterns exhausted the choice. They don't — CSS custom properties driven from a computed, classless inline style objects, component-scoped style blocks, and CSS-in-JS tokens are all reasonable in TKO. Section reframed as "two examples among several" under a "Reactive styling" heading, with the named parenthetical listing the other valid approaches. Emphasis stays on consistency within a codebase, not on picking a particular pattern. 2. The adversarial-review audit trail was documented as going into the PR description. That breaks the contract a PR description is meant to honour (why the change exists, what problem it solves) and feels like editing someone else's artefact for process metadata. Moved the audit line to the **commit message** that introduces the in-scope change, one line per in-scope commit. Explicit "do not add review outcomes to the PR description" clause in process.md. Squash-merge caveat added: copy audit lines into the squash target message manually if the merge rewrites commit messages. Also tightened wording consistency between AGENTS.md and process.md ("commit message that introduces the in-scope change" now appears in both). Adversarial pass: Explore subagent. Flagged 3 valid (commit- scope ambiguity, squash-merge vulnerability, cross-file wording drift) — all three addressed. Verdict after fixes: ship. Co-Authored-By: Claude Opus 4.7 (1M context) --- AGENTS.md | 2 +- tko.io/public/agents/guide.md | 20 ++++++++++---------- tko.io/public/agents/process.md | 5 +++-- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2273b38f..6307527e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -206,7 +206,7 @@ Avoid scope creep. If an improvement would balloon the PR, file a follow-up issu Before declaring a change done, steelman the case against it. Ask what could go wrong, what assumption could be false, what future goal it quietly forecloses, what coverage or signal it weakens, who it surprises. -**Adversarial review is mandatory for in-scope changes** (code, tests, public API, agent-facing docs, CI, `tools/build.ts`, `vitest.config.ts`, `biome.json`, landing commit messages). A single pair of eyes (yours) is not enough in a dark factory — the missing human reviewer has to be replaced by a second agent that was not told what "good" looks like and is asked only "where is this wrong?". Spawn a fresh subagent, brief it with the artefact + claim only (no author reasoning), bias toward flagging, verify any findings defensively, and record the outcome in the PR description. Out of scope: typos, whitespace, comment corrections. Full how-to and audit-trail format: [`tko.io/public/agents/process.md`](tko.io/public/agents/process.md#adversarial-review-is-mandatory). +**Adversarial review is mandatory for in-scope changes** (code, tests, public API, agent-facing docs, CI, `tools/build.ts`, `vitest.config.ts`, `biome.json`, landing commit messages). A single pair of eyes (yours) is not enough in a dark factory — the missing human reviewer has to be replaced by a second agent that was not told what "good" looks like and is asked only "where is this wrong?". Spawn a fresh subagent, brief it with the artefact + claim only (no author reasoning), bias toward flagging, verify any findings defensively, and record the outcome at the end of the commit message that introduces the in-scope change — one audit line per in-scope commit, never the PR description (that's for *why* the change exists, not reviewer ceremony). Out of scope: typos, whitespace, comment corrections. Full how-to and audit-trail format: [`tko.io/public/agents/process.md`](tko.io/public/agents/process.md#adversarial-review-is-mandatory). This is the ceiling on "Always Improve": that section pushes toward *more* in a PR; this one pushes toward *scrutiny* of what's in it. Use both — improve in scope, audit the scope itself here. diff --git a/tko.io/public/agents/guide.md b/tko.io/public/agents/guide.md index 28e882dc..fdfc0fcd 100644 --- a/tko.io/public/agents/guide.md +++ b/tko.io/public/agents/guide.md @@ -72,18 +72,18 @@ When the goal is to demonstrate TKO itself, keep the state flow inside observabl - If an example contrasts reactive models, the counters and highlighted state should also be observable-driven so the example demonstrates the pattern instead of bypassing it. - The line to avoid is using the DOM itself as the mutable source of truth after bindings are active. -### Reactive styling: two valid patterns +### Reactive styling -Reactive state that drives visual variations can be expressed two ways. Neither is universally "correct" — both produce the same runtime behaviour. **Pick one and apply it consistently across a codebase.** Mixing the two for the same kind of state is what hurts readability. +Reactive state that drives visual variations can be expressed several ways in TKO. Two common patterns are shown below; they are examples, not an exhaustive list (CSS custom properties written from a computed, classless inline style objects, component-scoped style blocks, and CSS-in-JS tokens are all reasonable too). **What matters more than the specific pattern is that a codebase applies one consistently** for the same kind of state — mixing approaches for identical problems is what hurts readability. -**Pattern A — computed class name / inline style.** The computed returns the exact class name or style string. Styling lives next to the reactive expression. +**Example A — computed class name / inline style.** The computed returns the class name or style string directly. The rule sits next to the reactive expression. ```tsx const className = this.computed(() => this.isActive() ? classes.active : classes.inactive) return
...
``` -**Pattern B — computed attribute, CSS selector does the switching.** The computed returns a flag value; CSS reads the attribute and applies styles. +**Example B — computed attribute, CSS selector does the switching.** The computed returns a flag value; CSS reads the attribute and applies styles. ```tsx const activeAttr = this.computed(() => this.isActive() || undefined) @@ -95,13 +95,13 @@ return
...
.my-component[data-active] { opacity: 1; } ``` -**Trade-offs.** -- Pattern A keeps the rule ("active → this class") visible in the component file; refactors that rename classes stay local. Split visibility if the class body lives in a separate stylesheet. -- Pattern B keeps styling in CSS and pairs naturally with the `|| undefined` pattern for binary attributes (see Gotchas); adding a new visual variant means editing CSS, not the component. -- Pattern B sidesteps class-name string concatenation inside computeds for complex multi-state styling. Pattern A can avoid the same footgun with template literals or a class-map helper; it just doesn't get it for free. -- Pattern A reads more directly when one class encapsulates many unrelated properties that Pattern B would need multiple attribute selectors to reach. +**Trade-offs between these two.** +- A keeps the rule ("active → this class") visible in the component file; refactors that rename classes stay local. Split visibility if the class body lives in a separate stylesheet. +- B keeps styling in CSS and pairs naturally with the `|| undefined` pattern for binary attributes (see Gotchas); adding a new visual variant means editing CSS, not the component. +- B sidesteps class-name string concatenation inside computeds for complex multi-state styling. A can avoid the same footgun with template literals or a class-map helper; it just doesn't get it for free. +- A reads more directly when one class encapsulates many unrelated properties that B would need multiple attribute selectors to reach. -Whichever pattern the codebase adopts, the corresponding classic `data-bind` writers are `css:` / `style:` for pattern A, or `attr:` for pattern B (`data-bind="attr: { 'data-active': activeAttr }"`). +Classic `data-bind` writers for the same effects: `css:` / `style:` for A, `attr:` for B (`data-bind="attr: { 'data-active': activeAttr }"`). ### JSX scope rule diff --git a/tko.io/public/agents/process.md b/tko.io/public/agents/process.md index d61986f0..a84fb7a8 100644 --- a/tko.io/public/agents/process.md +++ b/tko.io/public/agents/process.md @@ -58,10 +58,11 @@ looks like and is asked only "where is this wrong?". - Apply the AGENTS.md failure-modes list *and* the domain-specific checklist for the artefact type (docs → "Never ship docs that reference things that don't exist"; tests → disposal leaks + env assumptions; public API → backwards-compat + changeset; etc.). - If the reviewer returns a finding, **verify the specific claim** (re-run the command, read the named file, grep for the named symbol) — do not rely on your own prior reasoning to dismiss it. Subagents produce false positives, so verification is defensive, not a licence to override. If after verification you still believe the finding is wrong, record the reasoning in the PR description or as a code comment so a future reader (or maintainer) can judge it; do not silently reject. - If the pass surfaces a finding that belongs in a separate PR, file a follow-up or spawn a task rather than expanding the current change — keep "Keep PRs focused" intact. -- Record that the pass was run. A single line in the PR description is enough: +- Record that the pass was run. A single line at the end of the **commit message** that introduces the in-scope change is enough: `Adversarial pass: . Result: clean` or `Adversarial pass: . Flagged : . Resolved: .` - Without an audit trail, compliance is unverifiable and the rule is trivially gamed. + If a PR has multiple in-scope commits, each gets its own audit line; do not batch them. The commit message is the right home: it travels with the change in `git log` forever, stays granular to what was reviewed, and keeps process metadata out of the PR description (which is for *why* the change exists and *what* it does — not for reviewer ceremony). Do not add review outcomes to the PR description. Without *some* audit trail, compliance is unverifiable and the rule is trivially gamed; the commit message is a cheap, out-of-the-way place to leave it. + Caveat for squash-merge repos: squashing collapses per-commit audit lines into the squash target's message. That is acceptable as long as the lines survive the squash; if the squash message is auto-truncated or rewritten, copy the audit lines into it manually before merging. - Only after the pass returns clean (or returns findings that you have verified and addressed, deferred to a follow-up, or consciously rejected with documented reasoning) may you declare the work done. ## Credits