diff --git a/packages/agents/content/__tests__/injection-point-placement.unit.test.ts b/packages/agents/content/__tests__/injection-point-placement.unit.test.ts new file mode 100644 index 00000000..332ec430 --- /dev/null +++ b/packages/agents/content/__tests__/injection-point-placement.unit.test.ts @@ -0,0 +1,192 @@ +import { readdir, readFile } from 'node:fs/promises'; +import path from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { expandIncludes } from '../../src/lib/directive-expander.ts'; +import { isUnderTestDirectory } from '../../src/lib/fs-helpers.ts'; + +// An include and a guidance hook both splice content that brings its own headings, so a host heading deeper than the +// injected content's shallowest one renders as a subsection of the injection rather than of the host body. For a hook +// the parent is worse than misattributed: it is whichever rulebook the local binding supplied. +// +// The scan reads source rather than rendered output. Once includes expand, an inlined partial is byte-identical to the +// text around it, and no pass over the result can tell a host heading from an injected one. +const CONTENT_ROOT = new URL('../', import.meta.url).pathname; + +// Mirrors of the production grammars in `directive-expander.ts` and `guidance-hooks.ts`: each occupies a full line and +// tolerates surrounding whitespace, and self-close is tested before open so a path ending in `/` reads as a self-close. +const CLOSE_INCLUDE_REGEX = /^[ \t]*[ \t]*$/; +const FENCE_REGEX = /^\s*(`{3,}|~{3,})/; +const HEADING_REGEX = /^(#{1,6})\s/; +const HOOK_DIRECTIVE_REGEX = /^[ \t]*[ \t]*$/; +const OPEN_INCLUDE_REGEX = /^[ \t]*[ \t]*$/; +const SELF_CLOSE_INCLUDE_REGEX = /^[ \t]*[ \t]*$/; + +/** Level a bound rulebook's title lands at: `fillGuidanceHooks` demotes every heading in the fill by one. */ +const HOOK_FILL_LEVEL = 2; + +/** A heading the host body declares itself, outside any fence or slot region. */ +interface Heading { + readonly kind: 'heading'; + readonly lineNumber: number; + readonly level: number; + readonly text: string; +} + +/** A directive and the level its injected content starts at, against which a following host heading is judged. */ +interface Injection { + readonly kind: 'injection'; + readonly lineNumber: number; + readonly shallowestLevel: number; + readonly text: string; +} + +/** One source line that carries structure, paired with the 1-based line it occupies. */ +interface LiveLine { + readonly lineNumber: number; + readonly text: string; +} + +type Token = Heading | Injection; + +const levelByPartial = new Map(); + +describe('injection-point placement', () => { + it('no directive reparents the section following it', async () => { + const violations: Array = []; + for (const relativePath of await listContentMarkdown()) { + violations.push(...(await findViolations(relativePath))); + } + + expect( + violations, + `These directives inject content whose headings adopt the section below them:\n ${violations.join('\n ')}\n` + + `A host heading following a directive must sit at or above the injected content's shallowest heading level. ` + + `Promote the section, or move the directive below it.`, + ).toEqual([]); + }); +}); + +// region | Helpers + +/** + * Reports each directive in one body whose next host heading is deeper than the content it injects, rendered as one + * line per violation. Two directives sharing a following heading are both reported: each is independently misplaced. + */ +async function findViolations(relativePath: string): Promise> { + const violations: Array = []; + const open: Array = []; + + for (const token of await readStructure(relativePath)) { + if (token.kind === 'injection') { + open.push(token); + continue; + } + for (const injection of open) { + if (token.level > injection.shallowestLevel) { + violations.push( + `${relativePath}:${injection.lineNumber} ${injection.text} (injects h${injection.shallowestLevel}) ` + + `adopts ${relativePath}:${token.lineNumber} ${token.text}`, + ); + } + } + open.length = 0; + } + + return violations; +} + +/** Returns every Markdown file under `content/`, as paths relative to it. */ +async function listContentMarkdown(): Promise> { + const entries = await readdir(CONTENT_ROOT, { recursive: true }); + return entries.filter((entry) => entry.endsWith('.md') && !isUnderTestDirectory(entry)).toSorted(); +} + +/** + * Yields the lines of a body that carry structure, skipping every line inside a fenced block. A `#` inside a fence is + * content, so the fence is tracked rather than each line matched in isolation. A fenced directive is skipped for a + * different reason: the expander tracks no fences and still expands it, but the fence turns the headings it injects + * into literal text, which adopts nothing. + */ +function* readLiveLines(body: string): Generator { + let openFence: string | undefined; + for (const [index, text] of body.split('\n').entries()) { + const fence = FENCE_REGEX.exec(text)?.[1]; + if (openFence !== undefined) { + if (fence !== undefined && fence[0] === openFence[0] && fence.length >= openFence.length) { + openFence = undefined; + } + continue; + } + if (fence !== undefined) { + openFence = fence; + continue; + } + yield { lineNumber: index + 1, text }; + } +} + +/** + * Returns the shallowest level a partial contributes, or `undefined` when it contributes nothing a following section + * could nest under. A hook the partial declares counts at the fill level: hooks resolve after includes expand, so such + * a hook fills inside the host and can splice shallower than the partial's own headings. Memoized, since one partial + * reaches many consumers. + */ +async function readPartialLevel(partialPath: string): Promise { + if (!levelByPartial.has(partialPath)) { + const expanded = await expandIncludes(partialPath, CONTENT_ROOT); + let shallowest: number | undefined; + for (const { text } of readLiveLines(expanded)) { + const level = HOOK_DIRECTIVE_REGEX.test(text) ? HOOK_FILL_LEVEL : HEADING_REGEX.exec(text)?.[1]?.length; + if (level !== undefined && (shallowest === undefined || level < shallowest)) { + shallowest = level; + } + } + levelByPartial.set(partialPath, shallowest); + } + return levelByPartial.get(partialPath); +} + +/** + * Reads one body's headings and injection points in source order. Lines between an open include directive and its + * `` are slot content bound for the partial's ``, not structure of this body, and a + * partial that places that placeholder inside a fence turns them into example text — so they contribute no token. + */ +async function readStructure(relativePath: string): Promise> { + const filePath = path.join(CONTENT_ROOT, relativePath); + const body = await readFile(filePath, 'utf8'); + const tokens: Array = []; + let inSlot = false; + + for (const { lineNumber, text } of readLiveLines(body)) { + if (inSlot) { + inSlot = !CLOSE_INCLUDE_REGEX.test(text); + continue; + } + + const partial = SELF_CLOSE_INCLUDE_REGEX.exec(text)?.[1] ?? OPEN_INCLUDE_REGEX.exec(text)?.[1]; + if (partial !== undefined) { + inSlot = !SELF_CLOSE_INCLUDE_REGEX.test(text); + const shallowestLevel = await readPartialLevel(path.resolve(path.dirname(filePath), partial)); + if (shallowestLevel !== undefined) { + tokens.push({ kind: 'injection', lineNumber, shallowestLevel, text: text.trim() }); + } + continue; + } + + if (HOOK_DIRECTIVE_REGEX.test(text)) { + tokens.push({ kind: 'injection', lineNumber, shallowestLevel: HOOK_FILL_LEVEL, text: text.trim() }); + continue; + } + + const level = HEADING_REGEX.exec(text)?.[1]?.length; + if (level !== undefined) { + tokens.push({ kind: 'heading', lineNumber, level, text: text.trim() }); + } + } + + return tokens; +} + +// endregion | Helpers diff --git a/packages/agents/content/_partials/README.md b/packages/agents/content/_partials/README.md index 8b3dea73..2324fecc 100644 --- a/packages/agents/content/_partials/README.md +++ b/packages/agents/content/_partials/README.md @@ -37,6 +37,14 @@ Self-close is matched before open so that a path with a trailing slash is read c The `` placeholder is a partial-side directive. It appears at most once per partial. If a partial has no `` and the caller provides slot content, the expander throws `slot-without-children`. If a partial has `` and the caller provides no slot content (bare self-close, or empty open/close pair), the placeholder line is removed and surrounding lines join verbatim. +## Directive placement + +A host heading following a directive, deeper than the shallowest heading the injection contributes, renders as a subsection of the injected content rather than of the host body. Place every directive where the next host heading sits at or above that level. The `codeassembly-content-specification` rulebook carries the rule an author follows, under "Injection-point placement"; this section carries the level computation behind it. + +The deciding level is what the splice contributes, not a fixed `##`. A partial contributes its headings as authored -- `subagents/_partials/review-writes-scaffold.md` opens at `###`, so the `###` sections following it are its correct siblings -- and `##` for any guidance hook it declares, since hooks resolve after includes expand and so fill inside the host. A hook the host declares contributes `##` on its own, a bound rulebook's title being demoted one level to land there. + +Slot content is the caller's own text and contributes nothing here: a heading passed into a partial's `` is authored in the host beside the section that follows it, so its nesting is already visible where it is written. `content/__tests__/injection-point-placement.unit.test.ts` enforces the rule. + ## Path resolution Include paths are resolved relative to the directive-bearing file's directory in the source tree. The resolved target must remain inside `packages/agents/content/` (lexical containment is checked, not symlink resolution). Out-of-tree references throw `out-of-tree`. diff --git a/packages/agents/content/guidance/rulebooks/codeassembly-content-specification.md b/packages/agents/content/guidance/rulebooks/codeassembly-content-specification.md index 8b8bf1d2..fc202cea 100644 --- a/packages/agents/content/guidance/rulebooks/codeassembly-content-specification.md +++ b/packages/agents/content/guidance/rulebooks/codeassembly-content-specification.md @@ -144,3 +144,9 @@ Behavioural rules that govern an agent's output -- the recommendation gradient, Treat that restatement as load-bearing redundancy, not duplication. A DRY-driven refactor that strips the skill-local pointers and leaves only the global rule removes the mechanism by which the global rule takes effect. _(Enforced by `action-item-reinforcement.unit.test.ts` and `spec-inlining.unit.test.ts`.)_ Where a step's guidance is a matter of local taste rather than library doctrine -- a user's code-style preferences, a project's own glossary -- neither restatement above fits: a pointer sends the agent away to fetch the rule, and an inlined partial fixes one answer for every consumer at authoring time. Declare a guidance hook instead, ``, and leave the slot for a `codeassembly.yaml` to bind per project or per machine. An unbound hook contributes nothing to deployed output, so declaring one is safe wherever nothing fills it. A rulebook written for that slot declares `delivery: hook`, which records the route and lets `sync` report a binding and a delivery that disagree. The directive grammar is specified in `content/_partials/README.md` and the binding syntax in `packages/agents/README.md`. _(Convention; not enforced.)_ + +## Injection-point placement + +Injected content brings its own headings: a partial's as authored, and a guidance-hook fill's demoted one level, so a bound rulebook's title lands at `##`. A host heading following a directive that is deeper than the injected content's shallowest heading renders as a subsection of the injection rather than of the host -- and for a hook, under whichever rulebook the local binding supplied, so one body reads differently on two machines. + +Place every directive where the next host heading sits at or above that level. Where a section would otherwise nest, promote it or move the directive below it. The level that decides is what the injection contributes, not a fixed `##`: a partial opening at `###` and declaring no hook legitimately takes `###` siblings after it. _(Enforced by `injection-point-placement.unit.test.ts`.)_ diff --git a/packages/agents/content/skills/respond-to-review/SKILL.md b/packages/agents/content/skills/respond-to-review/SKILL.md index b87d80b2..223b7a9d 100644 --- a/packages/agents/content/skills/respond-to-review/SKILL.md +++ b/packages/agents/content/skills/respond-to-review/SKILL.md @@ -148,11 +148,11 @@ The disposition conflates two decisions: whether the change belongs (substance), > T1: ACCEPT. `createApiKey` is exported from the public `api/keys.ts` surface; the codebase's other public-API entry points (`createUser`, `createOrganization`) all validate their inputs at entry. The missing guard violates the established public-API invariant, so the change belongs. Timing is decided separately: The storage refactor's scope is otherwise tight, and adding the guard pulls in test fixtures unrelated to the refactor's purpose. The guard ships in a follow-up, and the follow-up ticket is filed now (per `create-tickets-immediately` guidance), not held as a maybe. - - -### Writing code after a review + + +## Writing code after a review Implementing an ACCEPTed finding puts you mid-conversation with the reviewer, and that is when the voice leaks. A comment must not narrate the change, retell the reviewer's concern, or cite a finding or acceptance-criterion ID. diff --git a/packages/agents/content/skills/revise-comments/SKILL.md b/packages/agents/content/skills/revise-comments/SKILL.md index c7293636..bd2745b7 100644 --- a/packages/agents/content/skills/revise-comments/SKILL.md +++ b/packages/agents/content/skills/revise-comments/SKILL.md @@ -32,7 +32,7 @@ Apply the comment-discipline audit to a target file set, editing comments in pla -### File-level carve-outs +## File-level carve-outs The carve-outs above apply to comments. Two file-level rules govern which lines this skill may touch at all: diff --git a/packages/agents/content/skills/testing-conventions/SKILL.md b/packages/agents/content/skills/testing-conventions/SKILL.md index 07be8656..b30b7516 100644 --- a/packages/agents/content/skills/testing-conventions/SKILL.md +++ b/packages/agents/content/skills/testing-conventions/SKILL.md @@ -68,7 +68,7 @@ Examples: -### Test files +## Comments in test files Test files are governed the same as source. Test names already communicate intent and assertions communicate the check, so everything outside the test-comment carve-out is over-commenting.