From 1d27cbf0dd3ef0e5ddc6a37ce9efe3b08c45e9a3 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 17 Mar 2026 19:39:44 -0400 Subject: [PATCH 1/3] feat: implement oxlint sensor spec v2 alignment corrections - Create .reviews/.oxlintrc.json sensor config with pedantic/style categories at warn level (14 bloat-relevant rules) - Add --config .reviews/.oxlintrc.json to all oxlint invocations across ReviewPR, AnalyzeRepo, AnalyzeRepoCI, and Doctor probe - Scope PR oxlint to changed files only via git diff --name-only + xargs for meaningful density metrics - Expand tsconfig.oxlint.json include paths in PR entry points to cover full monorepo (core, cli, durable-streams, durable-effects) - Add density calibration thresholds and Rule of Three to ExtraneousCodePolicy LLM prompt - Change diagnostics from optional to required in BloatPolicy, SlopPolicy, PrPolicyReport; remove defensive guard clauses - Add Rule of Three and YAGNI principles to RepoCleanupPolicy - Fix density precision to 3 decimal places in parseDiagnostics - Add prose narration to Doctor.md for local visibility - Create lint-plugins/no-scheme-specifiers.ts Deno lint plugin for jsr:/npm: bare specifier enforcement - Create .github/pull_request_template.md for process enforcement - Integrate oxlint sensor extension into code-review-agent spec --- .github/pull_request_template.md | 18 +++++ .reviews/.oxlintrc.json | 47 +++++++++++ .reviews/AnalyzeRepo.md | 4 +- .reviews/AnalyzeRepoCI.md | 4 +- .reviews/ReviewPR.local.md | 27 +++++-- .reviews/ReviewPR.md | 27 +++++-- .reviews/components/Doctor.md | 8 +- .reviews/components/PrPolicyReport.md | 8 +- .reviews/policies/BloatPolicy.md | 6 +- .reviews/policies/ExtraneousCodePolicy.md | 33 +++++--- .reviews/policies/RepoCleanupPolicy.md | 5 ++ .reviews/policies/SlopPolicy.md | 6 +- deno.json | 6 ++ lint-plugins/no-scheme-specifiers.ts | 48 +++++++++++ .../src/parse-diagnostics.ts | 4 +- .../tests/parse-diagnostics.test.ts | 2 +- specs/code-review-agent-spec.md | 79 +++++++++++++++++++ 17 files changed, 281 insertions(+), 51 deletions(-) create mode 100644 .github/pull_request_template.md create mode 100644 .reviews/.oxlintrc.json create mode 100644 lint-plugins/no-scheme-specifiers.ts diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..4facc31d8 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,18 @@ +## What does this PR do? + + +## Why? + + +## Scope confirmation +- [ ] All changed files relate to the stated purpose +- [ ] No drive-by refactors or "while I'm here" cleanups +- [ ] No formatting changes mixed with functional changes + +## New abstractions (if any) +- [ ] Each new type/interface/class has 3+ consumers or justification +- [ ] No speculative features ("we might need this later") + +## New dependencies (if any) +- Package: `name` +- Justification: diff --git a/.reviews/.oxlintrc.json b/.reviews/.oxlintrc.json new file mode 100644 index 000000000..274f333c3 --- /dev/null +++ b/.reviews/.oxlintrc.json @@ -0,0 +1,47 @@ +{ + "$schema": "./node_modules/oxlint/configuration_schema.json", + + "plugins": ["typescript", "unicorn", "import"], + + "categories": { + "correctness": "warn", + "suspicious": "warn", + "pedantic": "warn", + "style": "warn" + }, + + "rules": { + "eslint/no-unused-vars": ["warn", { + "args": "all", + "argsIgnorePattern": "^_", + "varsIgnorePattern": "^_", + "caughtErrorsIgnorePattern": "^_", + "destructuredArrayIgnorePattern": "^_", + "ignoreRestSiblings": true + }], + "typescript/no-inferrable-types": "warn", + "typescript/no-empty-object-type": "warn", + "typescript/no-useless-empty-export": "warn", + "typescript/no-unnecessary-type-constraint": "warn", + "typescript/no-unnecessary-parameter-property-assignment": "warn", + "unicorn/no-static-only-class": "warn", + "eslint/no-empty-function": "warn", + "eslint/no-console": ["warn", { "allow": ["warn", "error"] }], + "eslint/no-debugger": "warn", + + "typescript/no-unnecessary-type-arguments": "warn", + "typescript/no-unnecessary-type-assertion": "warn", + "typescript/no-redundant-type-constituents": "warn", + "typescript/no-unnecessary-boolean-literal-compare": "warn" + }, + + "overrides": [ + { + "files": ["**/*.test.ts", "**/*.spec.ts", "**/test/**"], + "rules": { + "eslint/no-console": "off", + "eslint/no-empty-function": "off" + } + } + ] +} diff --git a/.reviews/AnalyzeRepo.md b/.reviews/AnalyzeRepo.md index 99ea82713..bbb6bff87 100644 --- a/.reviews/AnalyzeRepo.md +++ b/.reviews/AnalyzeRepo.md @@ -82,7 +82,7 @@ const doctor = parseDoctorResult(doctorJson); || doctor.recommendation === "type-aware-filtered"}> ```bash exec -OUT=$(npx oxlint --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) +OUT=$(npx oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) if [ -n "$OUT" ]; then printf '%s' "$OUT" else @@ -96,7 +96,7 @@ fi && doctor.oxlintInstalled}> ```bash exec -OUT=$(npx oxlint --format json 2>/dev/null || true) +OUT=$(npx oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true) if [ -n "$OUT" ]; then printf '%s' "$OUT" else diff --git a/.reviews/AnalyzeRepoCI.md b/.reviews/AnalyzeRepoCI.md index 6f63e530d..d9ec92134 100644 --- a/.reviews/AnalyzeRepoCI.md +++ b/.reviews/AnalyzeRepoCI.md @@ -78,7 +78,7 @@ const doctor = parseDoctorResult(doctorJson); || doctor.recommendation === "type-aware-filtered"}> ```bash exec -OUT=$(npx oxlint --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) +OUT=$(npx oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) if [ -n "$OUT" ]; then printf '%s' "$OUT" else @@ -92,7 +92,7 @@ fi && doctor.oxlintInstalled}> ```bash exec -OUT=$(npx oxlint --format json 2>/dev/null || true) +OUT=$(npx oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true) if [ -n "$OUT" ]; then printf '%s' "$OUT" else diff --git a/.reviews/ReviewPR.local.md b/.reviews/ReviewPR.local.md index 764207e0f..73e561ff6 100644 --- a/.reviews/ReviewPR.local.md +++ b/.reviews/ReviewPR.local.md @@ -51,7 +51,14 @@ cat > .reviews/tsconfig.oxlint.json << 'TSCONFIG' "lib": ["ESNext", "DOM"], "types": [] }, - "include": ["packages/*/src/**/*.ts", "packages/*/*.ts"], + "include": [ + "packages/*/src/**/*.ts", + "packages/*/*.ts", + "core/src/**/*.ts", + "cli/src/**/*.ts", + "durable-streams/**/*.ts", + "durable-effects/**/*.ts" + ], "exclude": ["node_modules", "dist", ".vendor", "**/*.test.ts"] } TSCONFIG @@ -73,15 +80,22 @@ import { parseDoctorResult } from "@executablemd/code-review-agent"; const doctor = parseDoctorResult(doctorJson); ``` + + +```bash silent exec +git diff --name-only {BASE_SHA}...{HEAD_SHA} -- '*.ts' '*.tsx' | grep -v '\.test\.' | grep -v '\.spec\.' | grep -v '\.d\.ts$' | head -200 +``` + + + ```bash exec -OUT=$(npx oxlint --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) -if [ -n "$OUT" ]; then - printf '%s' "$OUT" +if [ -n "{changedTsFiles}" ]; then + echo "{changedTsFiles}" | tr '\n' ' ' | xargs npx oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true else echo "[]" fi @@ -93,9 +107,8 @@ fi && doctor.oxlintInstalled}> ```bash exec -OUT=$(npx oxlint --format json 2>/dev/null || true) -if [ -n "$OUT" ]; then - printf '%s' "$OUT" +if [ -n "{changedTsFiles}" ]; then + echo "{changedTsFiles}" | tr '\n' ' ' | xargs npx oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true else echo "[]" fi diff --git a/.reviews/ReviewPR.md b/.reviews/ReviewPR.md index 645100cb1..ab0c71458 100644 --- a/.reviews/ReviewPR.md +++ b/.reviews/ReviewPR.md @@ -59,7 +59,14 @@ cat > .reviews/tsconfig.oxlint.json << 'TSCONFIG' "lib": ["ESNext", "DOM"], "types": [] }, - "include": ["packages/*/src/**/*.ts", "packages/*/*.ts"], + "include": [ + "packages/*/src/**/*.ts", + "packages/*/*.ts", + "core/src/**/*.ts", + "cli/src/**/*.ts", + "durable-streams/**/*.ts", + "durable-effects/**/*.ts" + ], "exclude": ["node_modules", "dist", ".vendor", "**/*.test.ts"] } TSCONFIG @@ -77,15 +84,22 @@ import { parseDoctorResult } from "@executablemd/code-review-agent"; const doctor = parseDoctorResult(doctorJson); ``` + + +```bash silent exec +git diff --name-only {BASE_SHA}...{HEAD_SHA} -- '*.ts' '*.tsx' | grep -v '\.test\.' | grep -v '\.spec\.' | grep -v '\.d\.ts$' | head -200 +``` + + + ```bash exec -OUT=$(npx oxlint --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true) -if [ -n "$OUT" ]; then - printf '%s' "$OUT" +if [ -n "{changedTsFiles}" ]; then + echo "{changedTsFiles}" | tr '\n' ' ' | xargs npx oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig .reviews/tsconfig.oxlint.json --format json 2>/dev/null || true else echo "[]" fi @@ -97,9 +111,8 @@ fi && doctor.oxlintInstalled}> ```bash exec -OUT=$(npx oxlint --format json 2>/dev/null || true) -if [ -n "$OUT" ]; then - printf '%s' "$OUT" +if [ -n "{changedTsFiles}" ]; then + echo "{changedTsFiles}" | tr '\n' ' ' | xargs npx oxlint --config .reviews/.oxlintrc.json --format json 2>/dev/null || true else echo "[]" fi diff --git a/.reviews/components/Doctor.md b/.reviews/components/Doctor.md index ffa885b89..2e677069e 100644 --- a/.reviews/components/Doctor.md +++ b/.reviews/components/Doctor.md @@ -8,6 +8,8 @@ inputs: default: ".reviews/tsconfig.oxlint.json" --- +Checking environment for Oxlint static analysis... + ```bash exec @@ -50,6 +52,8 @@ const canProbeTypeAware = oxlintInstalled && tsgolintInstalled && nodeModulesExists && tsconfigExists; ``` +Scanning source files for scheme specifiers (jsr:, npm:)... + ```bash exec @@ -73,13 +77,15 @@ const jsrCount = specifierLines.filter(l => l.includes("jsr:")).length; const npmCount = specifierLines.filter(l => l.includes("npm:")).length; ``` +Running type-aware probe to test Oxlint compatibility... + ```bash exec -RESULT=$(npx oxlint --type-aware --tsconfig {tsconfigPath} --format json 2>.reviews/probe-stderr.tmp || true) +RESULT=$(npx oxlint --config .reviews/.oxlintrc.json --type-aware --tsconfig {tsconfigPath} --format json 2>.reviews/probe-stderr.tmp || true) STDERR=$(cat .reviews/probe-stderr.tmp 2>/dev/null || echo "") rm -f .reviews/probe-stderr.tmp echo "{\"diagnostics\":$RESULT,\"stderr\":\"$STDERR\"}" diff --git a/.reviews/components/PrPolicyReport.md b/.reviews/components/PrPolicyReport.md index 1e2ea2e4a..915898131 100644 --- a/.reviews/components/PrPolicyReport.md +++ b/.reviews/components/PrPolicyReport.md @@ -5,10 +5,10 @@ inputs: required: true diagnostics: type: object - required: false + required: true doctor: type: object - required: false + required: true --- ## PR #{pr.meta.number}: {pr.meta.title} @@ -21,10 +21,6 @@ inputs: - - - - diff --git a/.reviews/policies/BloatPolicy.md b/.reviews/policies/BloatPolicy.md index b2ad58ef1..344ba6144 100644 --- a/.reviews/policies/BloatPolicy.md +++ b/.reviews/policies/BloatPolicy.md @@ -5,7 +5,7 @@ inputs: required: true diagnostics: type: object - required: false + required: true --- @@ -39,11 +39,7 @@ inputs: severity="warning" message="{count} console statements." /> - 0}> - - - diff --git a/.reviews/policies/ExtraneousCodePolicy.md b/.reviews/policies/ExtraneousCodePolicy.md index 6a513a94e..b96cdebb7 100644 --- a/.reviews/policies/ExtraneousCodePolicy.md +++ b/.reviews/policies/ExtraneousCodePolicy.md @@ -5,13 +5,14 @@ inputs: required: true diagnostics: type: object - required: false + required: true doctor: type: object - required: false + required: true --- - + 20}> @@ -22,29 +23,35 @@ You are reviewing a TypeScript PR for EXTRANEOUS code only. PR: {pr.meta.title} Description: {pr.meta.body} - - STATIC ANALYSIS SIGNALS: {diagnostics.summary} Violation density: {diagnostics.density} per added line. -Interpret these signals in context. A few inferrable-type warnings -in a large PR are noise. But clusters of unused-vars + -empty-functions + unnecessary-type-assertions concentrated in the -same files suggest unreviewed generated code. +DENSITY CALIBRATION: +- Below 0.020: clean — experienced contributor, reviewed code +- 0.020–0.080: normal — minor issues, typical development +- Above 0.100: elevated — likely unreviewed generated code +Adjust judgment based on specific patterns, not number alone. - +INTERPRETATION RULES: +- A few inferrable-type warnings in a large PR are noise. +- Clusters of unused-vars + empty-functions + unnecessary-type- + assertions in the SAME FILES suggest unreviewed generated code. +- Files with both high comment density AND multiple Oxlint + structural violations are the strongest slop signal. Report ONLY: 1. Scope creep — changes unrelated to stated purpose -2. Speculative abstractions — new constructs with one consumer +2. Speculative abstractions — fewer than 3 consumers (Rule of + Three: don't abstract until third use) 3. Dead constructs — declarations never referenced in diff 4. Wrapper indirection — functions that only forward calls -5. Signal clusters — files where multiple Oxlint rules fire together +5. Signal clusters — files where multiple Oxlint rules fire +6. Object literal assertions — `{} as Type` hiding missing props Do NOT flag test helpers, exported types, or style preferences. -For each finding: FILE, PATTERN, CONCERN, QUESTION for the author. +For each finding: FILE, PATTERN, CONCERN, QUESTION for author. If clean: "No extraneous code patterns detected." diff --git a/.reviews/policies/RepoCleanupPolicy.md b/.reviews/policies/RepoCleanupPolicy.md index 1110c49fe..5b705b26e 100644 --- a/.reviews/policies/RepoCleanupPolicy.md +++ b/.reviews/policies/RepoCleanupPolicy.md @@ -29,6 +29,11 @@ Your job is NOT to re-analyze diagnostics. The ranking is already done. Your job IS to explain why each top cluster matters and what specific cleanup action to take. +PRINCIPLES: +- Rule of Three: flag any abstraction with fewer than 3 consumers. + Single-consumer abstractions should be inlined. +- YAGNI: flag code that exists "just in case" with no current caller. + {cleanupAnalysis.promptContext} For each of the top 5 clusters above, produce exactly this format: diff --git a/.reviews/policies/SlopPolicy.md b/.reviews/policies/SlopPolicy.md index b05aca157..155578c17 100644 --- a/.reviews/policies/SlopPolicy.md +++ b/.reviews/policies/SlopPolicy.md @@ -5,7 +5,7 @@ inputs: required: true diagnostics: type: object - required: false + required: true --- @@ -21,11 +21,7 @@ inputs: - 0}> - - - diff --git a/deno.json b/deno.json index 652199ef2..b51c74522 100644 --- a/deno.json +++ b/deno.json @@ -26,6 +26,12 @@ "oxlint": "npm:oxlint@^1", "oxlint-tsgolint": "npm:oxlint-tsgolint@^0.17" }, + "lint": { + "plugins": ["./lint-plugins/no-scheme-specifiers.ts"], + "rules": { + "include": ["no-scheme-specifiers/no-scheme-specifiers"] + } + }, "tasks": { "ema": "deno run --allow-all cli/src/cli.ts", "test": "deno test --allow-all core/tests/ durable-streams/tests/ durable-effects/tests/ packages/code-review-agent/tests/", diff --git a/lint-plugins/no-scheme-specifiers.ts b/lint-plugins/no-scheme-specifiers.ts new file mode 100644 index 000000000..d17da08ea --- /dev/null +++ b/lint-plugins/no-scheme-specifiers.ts @@ -0,0 +1,48 @@ +// deno-lint-ignore-file no-explicit-any +const plugin: Deno.lint.Plugin = { + name: "no-scheme-specifiers", + rules: { + "no-scheme-specifiers": { + create(context) { + function checkSource( + node: any, + ) { + if (!node.source) return; + const source = node.source.value; + if ( + typeof source === "string" + && (source.startsWith("jsr:") + || source.startsWith("npm:")) + ) { + const bare = source + .replace(/^jsr:/, "") + .replace(/^npm:/, "") + .replace(/@[\d^~>=<.*]+$/, ""); + + context.report({ + node: node.source, + message: + `Use bare specifier "${bare}" instead of ` + + `"${source}". Add "${bare}": "${source}" ` + + `to deno.json "imports".`, + fix(fixer) { + return fixer.replaceText( + node.source, + `"${bare}"`, + ); + }, + }); + } + } + + return { + ImportDeclaration: checkSource, + ExportNamedDeclaration: checkSource, + ExportAllDeclaration: checkSource, + }; + }, + }, + }, +}; + +export default plugin; diff --git a/packages/code-review-agent/src/parse-diagnostics.ts b/packages/code-review-agent/src/parse-diagnostics.ts index babf99c8d..1dfc1bb8f 100644 --- a/packages/code-review-agent/src/parse-diagnostics.ts +++ b/packages/code-review-agent/src/parse-diagnostics.ts @@ -177,14 +177,14 @@ export function parseDiagnostics( const fileCount = allFiles.size; const ruleCount = groups.length; const density = pr.stats.additions > 0 - ? Math.round((total / pr.stats.additions) * 100) / 100 + ? Math.round((total / pr.stats.additions) * 1000) / 1000 : 0; const lines: string[] = []; lines.push( `Oxlint: ${total} diagnostic${total !== 1 ? "s" : ""} across ${fileCount} file${fileCount !== 1 ? "s" : ""} (${ruleCount} rule${ruleCount !== 1 ? "s" : ""})`, ); - lines.push(`Density: ${density} violations/added-line`); + lines.push(`Density: ${density.toFixed(3)} violations/added-line`); lines.push(""); for (const g of groups) { diff --git a/packages/code-review-agent/tests/parse-diagnostics.test.ts b/packages/code-review-agent/tests/parse-diagnostics.test.ts index 10bf78363..b227e4a3a 100644 --- a/packages/code-review-agent/tests/parse-diagnostics.test.ts +++ b/packages/code-review-agent/tests/parse-diagnostics.test.ts @@ -325,7 +325,7 @@ describe("parseDiagnostics", () => { const result = parseDiagnostics(raw, makePR(100), makeDoctor()); expect(result.summary).toContain("3 diagnostics across 2 files (2 rules)"); - expect(result.summary).toContain("Density: 0.03 violations/added-line"); + expect(result.summary).toContain("Density: 0.030 violations/added-line"); expect(result.summary).toContain("no-unused-vars (2)"); expect(result.summary).toContain("no-console (1)"); }); diff --git a/specs/code-review-agent-spec.md b/specs/code-review-agent-spec.md index 8a901677d..7bd57d6dc 100644 --- a/specs/code-review-agent-spec.md +++ b/specs/code-review-agent-spec.md @@ -1166,3 +1166,82 @@ code-review-agent PR: - Phase 4: Rule components — code-review-agent PR - Phase 5: Policy documents + entry points + CI — code-review-agent PR - Phase 6: Sample modifier removal — PR #35 + +--- + +## 13. Extension: Oxlint Sensor (v2) + +**Full spec:** `oxlint-sensor-spec-v2.md` + +Oxlint runs as a structured signal source inside the review pipeline. +Its JSON output becomes a density metric the LLM uses alongside the +diff to detect quality deficits that correlate with unreviewed AI +output. All rules at `"warn"` — Oxlint collects signals, not verdicts. + +### 13.1 Sensor configuration + +`.reviews/.oxlintrc.json` — committed config with `pedantic: "warn"` +and `style: "warn"` enabled (14 bloat-relevant rules). All oxlint +invocations in capture blocks reference this config via +`--config .reviews/.oxlintrc.json`. + +### 13.2 Environment detection (`Doctor.md`) + +The Doctor component probes the environment before oxlint runs: +oxlint binary, tsgolint binary, `node_modules/`, tsconfig, scheme +specifier scan (`jsr:`, `npm:`), and a type-aware test run. Outputs +a recommendation: `type-aware`, `type-aware-filtered`, or +`syntax-only`. Includes prose narration for local visibility. + +### 13.3 PR-scoped analysis + +PR entry points (`ReviewPR.md`, `ReviewPR.local.md`) scope oxlint +to changed `.ts`/`.tsx` files only via `git diff --name-only` + +`xargs`. Density against `pr.stats.additions` is only meaningful +when diagnostics come from the same files the additions are in. +Repo analysis entry points run on everything. + +### 13.4 Density calibration + +| Density | Interpretation | +|---|---| +| < 0.020 | Clean — experienced contributor, reviewed code | +| 0.020–0.080 | Normal — minor issues, typical development | +| > 0.100 | Elevated — likely unreviewed generated code | + +3 decimal places to preserve signal in the narrow normal band. + +### 13.5 Policy updates + +- `ExtraneousCodePolicy.md` — density calibration thresholds, + interpretation rules, Rule of Three, and object literal assertion + detection added to LLM prompt +- `BloatPolicy.md`, `SlopPolicy.md` — `diagnostics` changed from + optional to required; defensive guards removed +- `RepoCleanupPolicy.md` — Rule of Three and YAGNI principles + added to LLM prompt + +### 13.6 Import specifier enforcement + +`lint-plugins/no-scheme-specifiers.ts` — Deno lint plugin that +flags `jsr:` and `npm:` scheme specifiers in source files and +auto-fixes them to bare specifiers. Required for tsgo/Oxlint +compatibility since tsgo uses Node module resolution. + +### 13.7 Process enforcement + +`.github/pull_request_template.md` — scope confirmation, Rule of +Three checklist for new abstractions, dependency justification. + +### 13.8 Additional files + +``` +.reviews/ + .oxlintrc.json Sensor config (committed) + +.github/ + pull_request_template.md Process enforcement + +lint-plugins/ + no-scheme-specifiers.ts Deno lint plugin +``` From 9086725e378af1e8f9ca3a42cd5b7f53a5035424 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 17 Mar 2026 20:05:42 -0400 Subject: [PATCH 2/3] feat: add CSS selector extraction to via remark + unist-util-select MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix Doctor.md JSON parsing failure in CI: prose narration text was included in the rendered output, causing parseDoctorResult to fall back to defaults (oxlintInstalled: false). Doctor.md now wraps its JSON return in a code fence, and all entry points use to extract just the JSON. The select prop parses rendered markdown via remark into an mdast tree, then queries it with unist-util-select using CSS selector syntax (e.g. code[lang=json], paragraph, heading[depth=1]). Falls back to full rendered content when no node matches. Also fix UnusedInDiff false positive on non-TS files by filtering pr.added to .ts/.tsx files before scanning for type/interface declarations. Updates executable-mdx-spec.md §1, §6.5 (Capture rules, select prop section, pseudocode, test table C29-C31, decisions 94-96) and code-review-agent-spec.md §13.2 (Doctor description). --- .reviews/AnalyzeRepo.md | 2 +- .reviews/AnalyzeRepoCI.md | 2 +- .reviews/ReviewPR.local.md | 2 +- .reviews/ReviewPR.md | 2 +- .reviews/components/Doctor.md | 2 +- .reviews/components/UnusedInDiff.md | 5 ++- core/deno.json | 5 ++- core/src/expand.ts | 33 ++++++++++++---- core/tests/expand.test.ts | 41 +++++++++++++++++++- deno.json | 4 +- deno.lock | 31 ++++++++++++++- specs/code-review-agent-spec.md | 3 ++ specs/executable-mdx-spec.md | 60 ++++++++++++++++++++++++++--- 13 files changed, 168 insertions(+), 24 deletions(-) diff --git a/.reviews/AnalyzeRepo.md b/.reviews/AnalyzeRepo.md index bbb6bff87..91cbda713 100644 --- a/.reviews/AnalyzeRepo.md +++ b/.reviews/AnalyzeRepo.md @@ -64,7 +64,7 @@ const pr = { }; ``` - + diff --git a/.reviews/AnalyzeRepoCI.md b/.reviews/AnalyzeRepoCI.md index d9ec92134..17311ac7d 100644 --- a/.reviews/AnalyzeRepoCI.md +++ b/.reviews/AnalyzeRepoCI.md @@ -60,7 +60,7 @@ const pr = { }; ``` - + diff --git a/.reviews/ReviewPR.local.md b/.reviews/ReviewPR.local.md index 73e561ff6..39508c10b 100644 --- a/.reviews/ReviewPR.local.md +++ b/.reviews/ReviewPR.local.md @@ -68,7 +68,7 @@ TSCONFIG ollama show qwen3:30b-a3b >/dev/null 2>&1 || ollama pull qwen3:30b-a3b ``` - + diff --git a/.reviews/ReviewPR.md b/.reviews/ReviewPR.md index ab0c71458..25c1a789d 100644 --- a/.reviews/ReviewPR.md +++ b/.reviews/ReviewPR.md @@ -72,7 +72,7 @@ cat > .reviews/tsconfig.oxlint.json << 'TSCONFIG' TSCONFIG ``` - + diff --git a/.reviews/components/Doctor.md b/.reviews/components/Doctor.md index 2e677069e..0072c9d38 100644 --- a/.reviews/components/Doctor.md +++ b/.reviews/components/Doctor.md @@ -176,5 +176,5 @@ const doctor = { }, }; -return JSON.stringify(doctor); +return '```json\n' + JSON.stringify(doctor) + '\n```'; ``` diff --git a/.reviews/components/UnusedInDiff.md b/.reviews/components/UnusedInDiff.md index 2860b818f..39c9cdb10 100644 --- a/.reviews/components/UnusedInDiff.md +++ b/.reviews/components/UnusedInDiff.md @@ -16,7 +16,10 @@ inputs: const declPattern = new RegExp( `(?:${construct})\\s+(\\w+)`, "g" ); -const source = pr.added.map(l => l.content).join("\n"); +const lines = pr.added.filter(l => + l.file.endsWith(".ts") || l.file.endsWith(".tsx") +); +const source = lines.map(l => l.content).join("\n"); const names = []; let match; diff --git a/core/deno.json b/core/deno.json index 8410d2248..81400a5d7 100644 --- a/core/deno.json +++ b/core/deno.json @@ -12,7 +12,10 @@ "magic-string": "npm:magic-string@^0.30.21", "marked": "npm:marked@^17.0.4", "marked-terminal": "npm:marked-terminal@^7.3.0", + "remark": "npm:remark@15", "remend": "npm:remend@^1.2.2", - "zod": "npm:zod@^4.3.6" + "zod": "npm:zod@^4.3.6", + "unist-util-select": "npm:unist-util-select@^5", + "mdast-util-to-string": "npm:mdast-util-to-string@^4" } } diff --git a/core/src/expand.ts b/core/src/expand.ts index b628b07a4..c93fdeefa 100644 --- a/core/src/expand.ts +++ b/core/src/expand.ts @@ -37,6 +37,9 @@ import { validateProps } from "./validate.ts"; import { healSegment } from "./heal.ts"; import { scanSegments } from "./scanner.ts"; import { renderSegments } from "./render.ts"; +import { remark } from "remark"; +import { select as cssSelect } from "unist-util-select"; +import { toString as mdastToString } from "mdast-util-to-string"; // --------------------------------------------------------------------------- // Block ID counter (spec §6.1) @@ -245,10 +248,10 @@ function* expandCapture( } const propNames = Object.keys(segment.props); - if (propNames.some((name) => name !== "as")) { + if (propNames.some((name) => name !== "as" && name !== "select")) { return { type: "error", - message: ' only accepts the "as" prop.', + message: ' only accepts "as" and "select" props.', source: "Capture", }; } @@ -262,11 +265,13 @@ function* expandCapture( source: "Capture", }; } - return { - type: "error", - message: ' only accepts the "as" prop.', - source: "Capture", - }; + if (!expressionNames.every((n) => n === "select")) { + return { + type: "error", + message: ' only accepts "as" and "select" props.', + source: "Capture", + }; + } } if (segment.props.as === undefined) { @@ -304,6 +309,18 @@ function* expandCapture( ); const rendered = renderSegments(expandedChildren).replace(/\s+$/, ""); + // Apply CSS selector if select prop is present (spec §6.5) + let captured = rendered; + const selectProp = segment.props.select as string | undefined; + if (typeof selectProp === "string" && selectProp.length > 0) { + const tree = remark().parse(captured); + // deno-lint-ignore no-explicit-any + const node = cssSelect(selectProp, tree as any); + if (node) { + captured = "value" in node ? String(node.value) : mdastToString(node); + } + } + const env = yield* EvalEnvCtx.get(); if (!env) { return { @@ -312,7 +329,7 @@ function* expandCapture( source: "Capture", }; } - env.values[bindingName] = rendered; + env.values[bindingName] = captured; return undefined; } diff --git a/core/tests/expand.test.ts b/core/tests/expand.test.ts index 75ddd6e4e..a5d17d18e 100644 --- a/core/tests/expand.test.ts +++ b/core/tests/expand.test.ts @@ -340,7 +340,46 @@ describe("expansion", () => { const segments = scanSegments("text"); const output = yield* expand(segments, ctx); expect(output).toContain("ERROR"); - expect(output).toContain("only accepts the \"as\" prop"); + expect(output).toContain('only accepts "as" and "select" props'); + }); + + it("Capture with select extracts code block by CSS selector", function*() { + const ctx = makeCtx({}); + const segments = scanSegments( + 'prose text\n\n```json\n{"key":"val"}\n```\n\nmore prose\n', + ); + const { output, env } = yield* expandWithEnv(segments, ctx); + expect(output).toBe(""); + expect(env["data"]).toBe('{"key":"val"}'); + }); + + it("Capture with select falls back to full content when no match", function*() { + const ctx = makeCtx({}); + const segments = scanSegments( + 'no code here\n', + ); + const { output, env } = yield* expandWithEnv(segments, ctx); + expect(output).toBe(""); + expect(env["data"]).toBe("no code here"); + }); + + it("Capture with select extracts paragraph text", function*() { + const ctx = makeCtx({}); + const segments = scanSegments( + 'Hello world\n', + ); + const { output, env } = yield* expandWithEnv(segments, ctx); + expect(output).toBe(""); + expect(env["data"]).toBe("Hello world"); + }); + + it("Capture accepts select alongside as without error", function*() { + const ctx = makeCtx({}); + const segments = scanSegments( + 'text\n', + ); + const output = yield* expand(segments, ctx); + expect(output).not.toContain("ERROR"); }); it("component as rejects expression prop", function*() { diff --git a/deno.json b/deno.json index b51c74522..efb2c1868 100644 --- a/deno.json +++ b/deno.json @@ -24,7 +24,9 @@ "@std/expect": "jsr:@std/expect@^1", "@std/testing/bdd": "jsr:@std/testing@^1/bdd", "oxlint": "npm:oxlint@^1", - "oxlint-tsgolint": "npm:oxlint-tsgolint@^0.17" + "oxlint-tsgolint": "npm:oxlint-tsgolint@^0.17", + "unist-util-select": "npm:unist-util-select@^5", + "mdast-util-to-string": "npm:mdast-util-to-string@^4" }, "lint": { "plugins": ["./lint-plugins/no-scheme-specifiers.ts"], diff --git a/deno.lock b/deno.lock index 080b3bdfd..30638acd9 100644 --- a/deno.lock +++ b/deno.lock @@ -27,11 +27,13 @@ "npm:magic-string@~0.30.21": "0.30.21", "npm:marked-terminal@^7.3.0": "7.3.0_marked@17.0.4", "npm:marked@^17.0.4": "17.0.4", + "npm:mdast-util-to-string@4": "4.0.0", "npm:oxlint-tsgolint@0.17": "0.17.0", "npm:oxlint@1": "1.56.0_oxlint-tsgolint@0.17.0", "npm:remark-mdx@3": "3.1.1", "npm:remark@15": "15.0.1", "npm:remend@^1.2.2": "1.2.2", + "npm:unist-util-select@5": "5.1.0", "npm:unist-util-visit@5": "5.1.0", "npm:zod@^4.3.6": "4.3.6" }, @@ -372,6 +374,9 @@ "bail@2.0.2": { "integrity": "sha512-0xO6mYd7JB2YesxDKplafRpsiOzPt9V02ddPCLbY1xYGPOX24NTyN50qnUxgCPcSoYMhKpAuBTjQoRZCAkUDRw==" }, + "boolbase@1.0.0": { + "integrity": "sha512-JZOSA7Mo9sNGB8+UjSgzdLtokWAky1zbztM3WRLCbZ70/3cTANmQmOdR7y2g+J0e2WXywy1yS468tY+IruqEww==" + }, "ccount@2.0.1": { "integrity": "sha512-eyrF0jiFpY+3drT6383f1qhkbGsLSifNAjA61IUjZjmLCWjItY6LB9ft9YhoDgwfmclB2zhu51Lc7+95b8NRAg==" }, @@ -446,6 +451,9 @@ "which" ] }, + "css-selector-parser@3.3.0": { + "integrity": "sha512-Y2asgMGFqJKF4fq4xHDSlFYIkeVfRsm69lQC1q9kbEsH5XtnINTMrweLkjYMeaUgiXBy/uvKeO/a1JHTNnmB2g==" + }, "ctrlc-windows@2.2.0": { "integrity": "sha512-t9y568r+T8FUuBaqKK60YGFJdj3b3ktdJW9WXIT3CuBdQhAOYdSZu75jFUN0Ay4Yz5HHicVQqAYCwcnqhOn23g==" }, @@ -969,6 +977,12 @@ "skin-tone" ] }, + "nth-check@2.1.1": { + "integrity": "sha512-lqjrjmaOoAnWfMmBPL+XNnynZh2+swxiX3WUE0s4yEHI6m+AwrK2UZOimIRl3X/4QctVqS8AiZjFqyOGrMXb/w==", + "dependencies": [ + "boolbase" + ] + }, "object-assign@4.1.1": { "integrity": "sha512-rJgTQnkUnH1sFw8yT6VSU3zD3sWmu6sZhIseY8VX+GRu3P6F7Fu+JNDoXfklElbLJSnc3FUQHVe4cU5hj+BcUg==" }, @@ -1200,6 +1214,16 @@ "@types/unist@3.0.3" ] }, + "unist-util-select@5.1.0": { + "integrity": "sha512-4A5mfokSHG/rNQ4g7gSbdEs+H586xyd24sdJqF1IWamqrLHvYb+DH48fzxowyOhOfK7YSqX+XlCojAyuuyyT2A==", + "dependencies": [ + "@types/unist@3.0.3", + "css-selector-parser", + "devlop", + "nth-check", + "zwitch" + ] + }, "unist-util-stringify-position@4.0.0": { "integrity": "sha512-0ASV06AAoKCDkS2+xw5RXJywruurpbC4JZSm7nr7MOt1ojAzvyyaO+UxZf18j8FCF6kmzCZKcAgN/yu2gm2XgQ==", "dependencies": [ @@ -1293,8 +1317,10 @@ "npm:@jsr/frontside__configliere@~0.2.3", "npm:@types/node@^24.5.2", "npm:effection@4.1.0-alpha.7", + "npm:mdast-util-to-string@4", "npm:oxlint-tsgolint@0.17", - "npm:oxlint@1" + "npm:oxlint@1", + "npm:unist-util-select@5" ], "members": { "cli": { @@ -1309,7 +1335,10 @@ "npm:magic-string@~0.30.21", "npm:marked-terminal@^7.3.0", "npm:marked@^17.0.4", + "npm:mdast-util-to-string@4", + "npm:remark@15", "npm:remend@^1.2.2", + "npm:unist-util-select@5", "npm:zod@^4.3.6" ] } diff --git a/specs/code-review-agent-spec.md b/specs/code-review-agent-spec.md index 7bd57d6dc..7d23fb377 100644 --- a/specs/code-review-agent-spec.md +++ b/specs/code-review-agent-spec.md @@ -1192,6 +1192,9 @@ oxlint binary, tsgolint binary, `node_modules/`, tsconfig, scheme specifier scan (`jsr:`, `npm:`), and a type-aware test run. Outputs a recommendation: `type-aware`, `type-aware-filtered`, or `syntax-only`. Includes prose narration for local visibility. +Its JSON output is wrapped in a `` ```json `` code fence and +extracted via `` (see EMA spec +§6.5), isolating the structured data from surrounding narration. ### 13.3 PR-scoped analysis diff --git a/specs/executable-mdx-spec.md b/specs/executable-mdx-spec.md index 8670449ad..26866f3bd 100644 --- a/specs/executable-mdx-spec.md +++ b/specs/executable-mdx-spec.md @@ -36,7 +36,9 @@ streaming, whitespace-normalized, ANSI-formatted output — see §9). Expansion also supports binding capture: component invocations may declare `as="name"` to route rendered output into `env.values` instead of the document, and the built-in `...` -directive captures inline rendered content into `env.values` without +directive captures inline rendered content into `env.values`, +optionally applying a CSS selector (via remark + `unist-util-select`) +to extract specific markdown nodes from the rendered content, without creating a new component boundary (see §6.5). ### 1.1 Example @@ -2439,9 +2441,18 @@ function* expandSegments( ctx, counter, ); - const rendered = renderSegments(expandedChildren).replace(/\s+$/, ""); + let captured = renderSegments(expandedChildren).replace(/\s+$/, ""); + + // If select prop is present, parse rendered content as markdown + // and apply CSS selector to extract matching node's text content. + // Falls back to full rendered content if no node matches. + // See §6.5 for selector syntax and node extraction rules. + if (segment.props.select is a non-empty string) { + captured = applyCssSelector(captured, segment.props.select); + } + const env = yield* ephemeral(EvalEnvCtx.expect()); - env.values[asBinding] = rendered; + env.values[asBinding] = captured; break; } @@ -2962,7 +2973,8 @@ Rules: - `as` is required and must be a valid identifier. - `` (self-closing) is invalid. - `` must have content. -- `` accepts no props other than `as`. +- `` accepts `as` (required) and `select` (optional) props. + No other props are allowed. - `as={expr}` is invalid (must be string literal). Behavior: @@ -2971,11 +2983,41 @@ Behavior: new `EvalScope`). 2. Render children to string. 3. Trim trailing whitespace (`/\s+$/`). -4. Store the string in `env.values[as]`. -5. Produce no output segment. +4. If `select` prop is present, apply CSS selector extraction (see below). +5. Store the resulting string in `env.values[as]`. +6. Produce no output segment. Overwrites are allowed for both mechanisms: last writer wins. +##### `select` prop — CSS selector extraction + +When the `select` prop is present, `` parses the rendered +children as markdown via `remark` and queries the AST with +`unist-util-select` using CSS selector syntax. The text content of the +first matching node is stored instead of the full rendered output. + +| Selector | Matches | +|---|---| +| `code` | Any fenced code block | +| `code[lang=json]` | Code block with `lang` attribute "json" | +| `heading[depth=1]` | h1 heading | +| `paragraph:first-child` | First paragraph | + +If no node matches the selector, the full rendered content is stored +(fallback behavior). + +For matched nodes, literal nodes (`Code`, `InlineCode`, `Html`, `Text`) +use their `.value` property directly. Parent nodes (e.g., `Paragraph`, +`Heading`) use `mdast-util-to-string` to extract concatenated child text. + +**Example.** A component that returns prose narration followed by +JSON wrapped in a `` ```json `` code fence. The caller uses +`` to extract +only the JSON value, ignoring the surrounding prose. If the +component later adds or removes narration text, the captured +binding is unaffected — the selector isolates the structured data +from the human-readable content. + #### Expression props Expression props pass runtime values from eval blocks to child @@ -4282,6 +4324,9 @@ instead of all under `root`). | C26 | Reserved prop `as` in inputs | Declaring `as` in component `inputs` fails frontmatter validation | | C27 | Invalid capture names | `as=""`, `as="123bad"`, or `as={expr}` produce validation errors | | C28 | `` invalid | Self-closing Capture produces ErrorSegment | +| C29 | `` CSS extraction | `` with code fence child stores code block value only | +| C30 | `` fallback | `select="code[lang=json]"` with no matching node stores full rendered content | +| C31 | `` paragraph | `select="paragraph"` extracts paragraph text content | ### Tier D — Code execution and modifier middleware @@ -4818,3 +4863,6 @@ A.md references . | 91 | Projected children carry caller's eval env | Children substituted via `` are tagged with `projectedEnv`. Expression props on projected children resolve against merged env (caller + component), with component bindings taking precedence. Follows React's lexical scoping model. | | 92 | Multi-level projection env propagation | When `expandComponent` receives `projectedEnv`, it merges it with the current context env before tagging the next level's children. Creates a cumulative chain: Root → Provider → Instruction → ReviewBody all carry root bindings. Innermost-wins on collision. | | 93 | AST-based user import extraction in eval blocks | `ImportDeclaration` nodes in eval blocks are extracted via acorn's `allowImportExportEverywhere` and hoisted to module top level by `compileBlock`. TypeScript `import type` normalized to spaces before parse, extracted from original source. | +| 94 | `` uses CSS selectors via remark + `unist-util-select` | Standard CSS selector syntax on markdown AST (mdast); reuses existing remark dependency; supports attribute selectors, combinators, pseudo-classes; matches Web platform conventions for querying tree structures | +| 95 | `select` falls back to full content on no match | Non-destructive — authors can add `select` to existing Captures without breaking behavior if the selector doesn't match; avoids silent data loss | +| 96 | Literal nodes use `.value`, parent nodes use `mdast-util-to-string` | Code blocks store text in `.value` (no child nodes); paragraphs/headings have child Text nodes requiring recursive extraction; two extraction strategies cover all mdast node types | From 1b85d1de79962a087f6af7c63ebd4dcccb5ed805 Mon Sep 17 00:00:00 2001 From: Taras Mankovski Date: Tue, 17 Mar 2026 20:10:55 -0400 Subject: [PATCH 3/3] test: add Capture select smoke test coverage Add to the smoke test document's Binding Capture section. The test captures a JSON array from a code fence surrounded by prose text, verifies the selector extracts only the code block value, and confirms the prose is not in the output. Uses printf to avoid bash quote stripping on the interpolated JSON. --- core/tests/smoke.test.ts | 7 +++++++ smoke-test/README.md | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/core/tests/smoke.test.ts b/core/tests/smoke.test.ts index e1973523e..add0deb41 100644 --- a/core/tests/smoke.test.ts +++ b/core/tests/smoke.test.ts @@ -105,6 +105,12 @@ describe("smoke test", { sanitizeOps: false, sanitizeResources: false }, () => { expect(output).toContain("| inline binding from Capture"); expect(output).not.toContain("Hidden capture should not render inline."); + // ----- Capture with CSS selector ----- + // select="code[lang=json]" extracts only the JSON code block value, + // ignoring surrounding prose text + expect(output).toContain('Selected JSON: ["alpha","bravo",42]'); + expect(output).not.toContain("Some prose before the data"); + // ----- In-Process Evaluation section ----- expect(output).toContain("§ In-Process Evaluation"); @@ -162,6 +168,7 @@ describe("smoke test", { sanitizeOps: false, sanitizeResources: false }, () => { expect(output).toContain("composable instructions"); expect(output).toContain("component as capture"); expect(output).toContain("Capture directive"); + expect(output).toContain("Capture select"); // ----- Instruction Component section ----- expect(output).toContain("§ Instruction Component"); diff --git a/smoke-test/README.md b/smoke-test/README.md index 8056a1d94..30a8b3863 100644 --- a/smoke-test/README.md +++ b/smoke-test/README.md @@ -166,6 +166,22 @@ Captured bindings are available to later executable blocks: echo "Capture values: {capturedFromComponent} | {capturedInline}" ``` +Capture with CSS selector extracts specific content from rendered output: + + +Some prose before the data. + +```json +["alpha","bravo",42] +``` + +More prose after. + + +```bash exec +printf 'Selected JSON: %s\n' '{capturedJson}' +``` + This Note is captured but intentionally not rendered inline: @@ -458,6 +474,7 @@ cat <<'EOF' | Expression props | | | component as capture | ... | | Capture directive | ... | +| Capture select | ... | | Durability | Timestamp stable across reruns | | eval modifier | js eval blocks with shared bindings | | persist modifier | js persist eval block, resource lifetime|