feat(#11): add context file validation script#87
Conversation
…EMPLATE Add npm run validate-context to validate agent-memory context files against the schema. Updated schema.json frontmatter definition to match TEMPLATE.md fields (id, category, domain, issueNumber, issueUrl, title, lastUpdated, summary, services, techStack). All 10 existing context files pass validation.
Hareet
left a comment
There was a problem hiding this comment.
Background
Based on the memory-distillation-pipeline, we need to converge our schema.json and template.md with the human-reviewed. There's a few good sections inside the distillation-pipeline that would be beneficial to us. I'll have a comment on that PR with some updated requests. I've ran the memory-distillation-pipeline for 1k PRs, and am trying to combine the format to open up per-domain bulk context PRs.
Review hiccups across 2 PRs that we could handle here:
Thanks for picking up the schema/TEMPLATE.md alignment here — a couple of things to reconcile before this lands, since it overlaps the provenance schema work in #109:
Date-field conflict with #109. This PR aligns schema.json to TEMPLATE.md using lastUpdated (camelCase) and requires it; #109 adds provenance fields using last_updated (snake_case). As written, whichever of the two merges second will leave one population of files failing validation. Could you reconcile the two here — e.g. accept the date under either name via an object-level anyOf ([{required:[last_updated]},{required:[lastUpdated]}]) and pick one as canonical going forward — so the schema is coherent once both land?
A few related divergences worth folding in while you're here: category appears as both improvement and enhancement across the corpus (treat as synonyms?), and the interoperability files use cht-interoperability-<n> ids, so an id pattern of ^cht-core-... would reject them — ^cht-[a-z0-9-]+-[0-9]+$ covers both.
Validation coverage. The "all existing context files pass" check looks like it ran against ~10 files; validating the full domains/ set surfaces a few real data issues the schema should catch: 4 files carry techStack values in services: (config, xml, xlsform, typescript) and use shared-libs (decide whether shared-libs joins CHTService or those get corrected), and 10729-smsparser-bugs-typos.md has an unquoted summary: with a colon that's invalid YAML. Might be worth running the new validate-context script across all of domains/ to confirm the full corpus is green.
…ation-script # Conflicts: # package.json
Adopts #87's converged camelCase schema.json and schema-utils.ts verbatim so the two PRs share one frontmatter contract and the 30 committed agent-memory files validate natively (no lastUpdated->last_updated alias). Distiller changes: - emit camelCase frontmatter (lastUpdated, issueNumber, issueUrl) plus the now-required services and techStack fields - derive issueNumber/issueUrl/id from the first linked issue, falling back to the PR number when the PR closes no issue - serialize frontmatter with js-yaml (correct quoting; fixes unquoted title) and AJV-validate every draft before writing — malformed drafts route to _skipped.ndjson as flag-for-human instead of landing in _pending/ Addresses the review's blocking schema-reconciliation item.
…, CLI, and CI workflow (#128) * feat(#108): add pipeline directory scaffolding and types - Create agent-memory/_pending/<domain>/ for all 8 CHT domains - Add empty agent-memory/_skipped.ndjson for filter audit log - Define pipeline types in src/types/pipeline.ts: ScrapedPR, LinkedIssue, ReviewComment, FilterDecision, SkipLogEntry, ScraperError * feat(#108): implement PR scraper using gh CLI Synchronous scraper that collects structured data from medic/cht-core PRs for the memory distillation pipeline. - Fetches PR metadata, diff, review summaries, and linked issues - Uses execFileSync with array args (no shell injection risk) - 50 MB maxBuffer with ENOBUFS → ScraperError - Graceful fallback for org-membership (read:org not required) - Linked issues extracted via regex; GitHub sidebar links documented as a known limitation (requires GraphQL) - PENDING reviews filtered out - 22 unit tests, all coverage thresholds met * feat(#108): extend schema with provenance fields and add validator - Add 10 optional frontmatter fields to schema.json: source_pr, source_sha, distilled_at, reviewed_by, reviewed_at, confidence, entities, concepts, related_issues, stale - All new fields are optional (backward-compatible with existing entries) - Update TEMPLATE.md with commented stubs and field reference table - Add src/scripts/validate-schema.ts (gray-matter + ajv) to validate all agent-memory /**/*.md frontmatter; 30/30 existing entries pass - Add npm run validate-schema script - Fix YAML syntax error in 10729-smsparser-bugs-typos.md (unquoted colon) * fix(#108): extend linked-issue regex to match full GitHub issue URLs Bare keyword regex (e.g. "Fixes #123") missed the URL form used by some cht-core PRs ("Fixes https://github.com/…/issues/123"). Updated pattern captures both forms and deduplicates across them. Added 2 tests: URL extraction and bare+URL dedup. * feat(#108): add filter stage with deterministic rules and LLM triage Three-stage pipeline filter for ScrapedPR objects: - Stage 1: 5 deterministic skip rules (bot, revert, chore/docs/ci/build, lockfile-only, translation-only) - Stage 2: 3 deterministic distill rules (bug+issue+multiservice, feature+issue, shared-libs+multiconsumer) - Stage 3: LLM triage via ChatAnthropic with Zod structured output; graceful fallback to flag-for-human on failure or missing API key Also adds ScrapedPR.author field, FilterResult/FilterOptions types, and _skipped.ndjson audit log (append-only, skip/flag decisions only). DoD items verified: - All 5 skip rules + all 3 distill rules implemented and tested - touchesMultipleServices edge cases covered - LLM failure -> flag-for-human, no throw - skipLlm option bypasses LLM call - 211 tests passing, branches 89.24%, lines 99.29% * feat(#108): add OpenRouter support with Anthropic fallback to filter triage Switch LLM triage from hardcoded ChatAnthropic to a dual-provider setup: - OpenRouter (OPENROUTER_API_KEY) as primary via ChatOpenAI - Anthropic direct (ANTHROPIC_API_KEY) as fallback - Graceful flag-for-human when neither key is set Also extracts DEFAULT_TRIAGE_MODEL constant and caches the triage chain via a module-level singleton (getTriageChain) to avoid re-construction on every PR in a batch run. TRIAGE_MODEL env var overrides the default (anthropic/claude-haiku-4). * fix(#108): correct OpenRouter model ID and add smoke-test script 'anthropic/claude-haiku-4' is not a valid OpenRouter model ID — updated DEFAULT_TRIAGE_MODEL to 'anthropic/claude-haiku-4-5'. Also adds smoke-test.ts for manual end-to-end pipeline validation. * feat(#108): add distiller stage — LLM-powered knowledge draft generator Implements distillPR() which takes a ScrapedPR that passed the filter stage and produces a schema-valid markdown draft in agent-memory/_pending/<domain>/. - Structured LLM output via Zod (domain enum, required fields) with OpenRouter-primary / Anthropic fallback, matching filter.ts pattern - Deterministic markdown assembly in code; AJV validates frontmatter before the file is written - distillFn injection option for tests (no real API calls in test suite) - Graceful degradation: LLM failure → flag-for-human + _skipped.ndjson - 25 tests, 86.31% branch coverage, all thresholds met * refactor(#108): simplify distiller — fix bugs, extract shared constants, async I/O - Fix double-wrapped error prefix in distillPR catch block (was emitting "Distill LLM unavailable: Distill LLM unavailable: ..." on the no-key path) - Replace sync fs.*Sync calls with await fs.promises.* so batch processing does not block the event loop - Fix .slice-before-filter on review comments so non-empty reviews beyond index 2 are not silently dropped - Derive CHT_DOMAINS list in buildPrompt from the constant rather than a separate hardcoded string that could drift - Extract CHT_DOMAINS, DEFAULT_PIPELINE_LOG_PATH, DEFAULT_PIPELINE_OUTPUT_DIR to src/constants/index.ts; both filter.ts and distiller.ts now import them - Switch process.env bracket notation to dot notation in both scripts - Remove redundant `const parsed = matter` alias in distiller.spec.ts * feat(#108): extend smoke-test to run distiller stage end-to-end Adds distillPR call after a distill decision so smoke-test validates the full scrape → filter → distill pipeline in one run. * test: reproduce bug — second call with same draft content rejected as missing frontmatter * fix: replace gray-matter .matter property check with hasFrontmatter() string check gray-matter v4 caches parsed results in matter.cache[input]. When the same content string is parsed a second time the cached object is returned, which may not retain the 'matter' non-enumerable property. Checking the raw string with hasFrontmatter() before calling matter() is deterministic and cache- independent, making all apply-mode tests reliable. * fix: use dot notation for Record<string,unknown> property access * feat(#108): add OpenReviewOptions/ReviewPRResult types and open-review-pr npm script * docs(#108): document OPENROUTER_API_KEY, TRIAGE_MODEL, DISTILL_MODEL in .env.example * chore: update skipped log with smoke-test run entries * refactor(#108): extract shared schema-utils and clean up open-review-pr - Extract buildValidator, normalizeFrontmatter, hasFrontmatter, REPO_ROOT, SCHEMA_PATH into src/scripts/schema-utils.ts; import from both open-review-pr.ts and validate-schema.ts to eliminate duplication - Fix buggy parsed.matter check in validate-schema.ts to use hasFrontmatter() - Hoist buildValidator() call to module scope in open-review-pr.ts so schema is compiled once per process, not on every openReviewPR() invocation - Replace existsSync+readdirSync TOCTOU pattern with single readdirSync+catch - Deduplicate path.basename call in writeSkipEntry - Bound uniqueBranchName loop to 99 iterations with explicit throw on exhaust - Remove phase 1/2/3 what-not-why comments * feat(#108): add run-pipeline CLI, GitHub Actions workflow, and skipped log update - src/scripts/run-pipeline.ts: CLI entry point wiring Scraper → Filter → Distiller for single PR (--pr) or recent window (--since / default 24h) - .github/workflows/run-pipeline.yml: daily cron (06:00 UTC) + manual dispatch that runs the pipeline and commits knowledge drafts back to the branch - package.json: add run-pipeline npm script - agent-memory/_skipped.ndjson: update with latest smoke-test run entries * fix(#108): resolve CI lint failures — promote ajv/gray-matter to deps, fix auto-fixable errors - Move ajv, ajv-formats, gray-matter from devDependencies to dependencies so n/no-unpublished-require passes for src/scripts/ files - Add missing ajv to dependencies (only ajv-formats was listed) - Run eslint --fix to correct dot-notation and indent violations - Remove unused callCount variable in open-review-pr.spec.ts - Remove invalid @typescript-eslint/no-throw-literal eslint-disable comments in scraper.spec.ts (rule was renamed in typescript-eslint v6+) * test(#108): add branch-coverage tests to meet 85% threshold - Add normalizeFrontmatter lastUpdated→last_updated aliasing test (covers schema-utils.ts lines 49-50) - Add non-Error LLM throw test in distillPR (covers distiller.ts line 275 false branch) - Add empty linkedIssues prompt test (issueContext false branch) - Add undefined prBody prompt test (null-coalescing branch) Branch coverage: 83.53% → 85.06% (279/328) * fix(#108): resolve all SonarCloud quality gate failures Cognitive complexity reductions (max 5 per function): - smoke-test: extract processPR helper (complexity 11→1) - run-pipeline: extract processSinglePR + errorMessage (12→3, 8→3) - open-review-pr: extract parseDraft, findValidDrafts, collectValidPlans, buildDryRunResults, promoteDomain, executeApply (26→1) - filter: extract runLlmTriage + use Set for labels (7→2, 6→3) - scraper: extract fetchMetadata, fetchDiff, fetchReviews (13→2) - validate-schema: extract processEntry + logFileResult (8→4, 9→2) Code quality warnings: - Replace child_process with node:child_process in 3 files - Use import matter from 'gray-matter' (remove unnecessary require assertions) - Use String#endsWith for bot check, RegExp#exec for match - Number.parseInt over parseInt throughout - String#replaceAll over regex replace (distiller, open-review-pr) - Flip negated condition in run-pipeline IIFE - String(fm.source_pr) to prevent [object Object] in template - files.toSorted() over files.sort() in validate-schema - Update scraper.spec.ts proxyquire key for node:child_process Coverage: 83.53% → 85.41% (281/329 branches) * fix(#108): resolve remaining 7 SonarCloud issues - smoke-test: extract filterAndDistill helper (processPR complexity 6→3) + prefer node:os import - filter: extract lockfile/translation booleans in checkSkipRules (7→5) + extract isBugWithLinkedIssueAndMultiService/isFeatureWithLinkedIssue/ isSharedLibsWithMultiService helpers for checkDistillRules (7→3) - open-review-pr: pre-compute sourcePrStr as typed string to prevent [object Object] template literal stringification - validate-schema: add localeCompare comparator to toSorted() * fix(#108): extract checkSkipRules helpers to eliminate && complexity (7→5) * fix(#108): resolve S5852 ReDoS in LOCKFILE_PATTERN regex Replace unbounded .* with [^/]* in the lockfile alternation so the wildcard cannot overlap the (?:^|/) anchor. Eliminates super-linear backtracking (50k-char non-match: ~0.07ms) while preserving match behavior across root-level and nested *.lock paths. * fix(#108): use mkdtempSync for smoke-test temp paths Resolves SonarCloud hotspots flagging predictable temp paths in a publicly-writable directory. mkdtempSync creates a per-run dir with a random name and 0700 permissions. * feat(#108): reconcile frontmatter schema with #87 (camelCase) Adopts #87's converged camelCase schema.json and schema-utils.ts verbatim so the two PRs share one frontmatter contract and the 30 committed agent-memory files validate natively (no lastUpdated->last_updated alias). Distiller changes: - emit camelCase frontmatter (lastUpdated, issueNumber, issueUrl) plus the now-required services and techStack fields - derive issueNumber/issueUrl/id from the first linked issue, falling back to the PR number when the PR closes no issue - serialize frontmatter with js-yaml (correct quoting; fixes unquoted title) and AJV-validate every draft before writing — malformed drafts route to _skipped.ndjson as flag-for-human instead of landing in _pending/ Addresses the review's blocking schema-reconciliation item. * chore(#108): reset _skipped.ndjson to empty (drop dev-run log noise) * docs(#108): document OpenRouter provider-agnostic rationale * fix(#108): address review nits in pipeline stages - filter: narrow LOCKFILE_PATTERN to known lockfile names (no longer skips unrelated *.lock files); pattern is now fully literal (no ReDoS surface) - filter: drop redundant inner try/catch in llmTriage; runLlmTriage is the single error boundary (covers default + injected triageFn paths) - scraper: drop unresolvable linked issues instead of returning empty stubs, so a 404'd reference can't flip a filter skip into a distill - scraper: wrap metadata/reviews JSON.parse in ScraperError (carries prNumber) instead of leaking a raw SyntaxError - distiller: slugify falls back to 'untitled' for symbol-only/non-Latin titles - open-review-pr: escape Markdown special chars in draft titles in the PR body - tests: relabel mislabeled single-service filter test; add coverage for all of the above (359 passing) * fix(#108): address secondary review follow-ups 1-3 - scraper: fetch reviews with `--paginate --slurp` and flatten the array-of-pages instead of string-stitching `[...][...]` — no longer corrupts review bodies containing `] [` or breaks on empty pages - open-review-pr: isolate promotion per domain — a domain failure is recorded as a 'failed' ReviewPRResult and the rest still run; clean up the orphan remote branch when PR creation fails after push - filter tests: stub both LLM providers (ChatOpenAI + ChatAnthropic) and clear both API keys per-test, removing cross-spec env leakage; add an explicit OpenRouter primary-path test 363 passing, 86% branch coverage * fix(#108): address secondary review follow-ups 4-7 - run-pipeline: validate --since/--pr as positive integers (error instead of silently processing nothing); guard the CLI IIFE behind require.main so the module is importable, and cover its orchestration functions (now 100%). - scraper: handle reviews from deleted GitHub accounts (user: null) by falling back to the 'ghost' author instead of dereferencing null. - open-review-pr: resolve source_pr 'owner/repo#N' refs to working /pull/N URLs via a new sourcePrUrl helper, instead of a dead repo-home anchor. - package.json: add the smoke-test npm script. Tests: 380 passing; coverage 97.21% stmts / 86.65% branch (>= 95% gate). * fix(#108): reject partially-numeric --since/--pr (roborev MEDIUM) Number.parseInt silently truncated '123abc' -> 123 and '1.5' -> 1, so partially-numeric flag values passed the positive-integer check. Validate the raw string with /^[1-9]\d*$/ before parsing, via a parsePositiveIntArg helper. Adds regression tests for trailing characters and decimals. * fix(#108): reject oversized --since/--pr values (roborev LOW) A digit string large enough to overflow Number.parseInt passed the regex but produced a non-safe-integer (and an invalid lookback date downstream). Parse with Number() and require Number.isSafeInteger. Adds a regression test. * refactor(#108): clear SonarCloud findings on PR #109 new code - run-pipeline: use Number.NaN over NaN (S7773); throw TypeError for the argument-validation guard (S7786). - open-review-pr: use String.raw for the Markdown-escape replacement (S7780); RegExp.exec() over String#match in sourcePrUrl (S6594); reduce cognitive complexity of promoteDomain and executeApply (S3776) by extracting stageDrafts, deleteRemoteBranch, and promoteDomainSafely helpers. Behavior unchanged; 384 passing, coverage 97.2% (>= 95% gate). * fix(#108): restore missing JSDoc opening tag in constants/index.ts A `/**` line was dropped (likely during rebase), leaving the comment body as bare tokens that TypeScript parsed as code — causing 30+ TS errors and the "Build, lint, and test" CI failure.
…tructure domain, concurrency (#119) * feat(#108): add Claude CLI provider and LLM rate-limit/auth batch-stop LLM_PROVIDER=claude-cli runs filter/distill via 'claude -p' on the operator's subscription (no API key). isRateLimitError/isAuthError/isBatchFatalError detect global LLM failures so the runner can stop the batch instead of flagging each PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#108): concurrency + resumable runner; stop batch on rate-limit/auth --concurrency (PIPELINE_CONCURRENCY, [1,10]) worker pool; --last removes the ~100-PR per-run cap; --resume skips processed PRs; --force bypasses filter. Rate-limit/auth errors stop the batch (exit 2) with a resume hint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#108): validate-schema --pending-only/--include-pending; wire CI gate; pin Node 22 CI validates the domains/ corpus on PRs and fresh drafts in the pipeline run. .env.example set for claude-cli + Opus 4.8/max + concurrency 4; .nvmrc=22.18.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#108): containerized seeder (vendored base sandbox from #114) + runbook Self-contained seeder image (claude -p + gh, read-only GH_TOKEN). Base docker tree vendored from #114 to build standalone; dedupe once #114 lands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(memory): correct invalid services values in 4 contacts files (overlaps #87) Drop non-service values (shared-libs/typescript) from services; config->webapp on 9915 so the corpus passes validate-schema. Reconcile with #87 at merge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#108): address review — validate --repo, guard stdio, add infrastructure to VALID_DOMAINS Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(#108): single-source the CHT taxonomy; lock schema/const drift with a test Derive CHTDomain/CHTService/CHTWorkflow from the CHT_DOMAINS/CHT_SERVICES/CHT_WORKFLOWS const arrays; pipeline.ts re-exports instead of re-declaring; distiller imports CHT_SERVICES. New test asserts schema.json enums == the const arrays. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(#108): add domainFit, related_workflows, and infrastructure domain schema.json enums, doc-search Record maps, TEMPLATE.md, and distiller/domain-inference tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#108): derive VALID_DOMAINS from CHT_DOMAINS so infrastructure isn't rejected Both VALID_DOMAINS hand-lists now derive from CHT_DOMAINS; updates the validator test message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#108): clear SonarCloud findings (regex backtracking, CC, assertions, re-export) Shared linear JSON extractor; CC refactors + justified NOSONAR; more Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#108): reduce cognitive complexity below 5 by refactoring (NOSONAR not honored) Extracted helpers; SonarCloud Automatic Analysis ignores the NOSONAR block, so refactored to <=5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#108): bundle worker-pool args into BatchCtx (resolve too-many-parameters) logPrStart/processBatchItem/runWorker now take a shared BatchCtx, each <=4 params; CC still <=5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#108): cover run-pipeline.ts (96-100%) and drop it from the coverage exclude Proxyquire tests for the run-pipeline helpers + concurrency>1; export 3 helpers for testing. Only providers/open-review-pr/domain-inference stay excluded; coverage gate green Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#108): use specific chai assertion for exitCode (SonarCloud S2701) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(#119): address review — domain roster, plain-text limit, force+resume Derive the inference prompt's domain roster/count from CHT_DOMAINS (was hardcoded 7, now 9). Classify plain-text usage-limit/auth CLI output as batch-fatal instead of a success result. Exempt explicitly named --pr from the --resume skip when --force is set (filterResumable). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#119): drop providers/** coverage exclusion; cover claude-cli + filterResumable claude-cli.ts 81%->96% (limit/auth, empty-stdout, spawn-error, malformed-envelope paths). istanbul-ignore only timer/signal callbacks; global gate 97/86/97/97 with providers in scope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(#119): note seeder claude-config volume only seeds on first creation Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(#119): gate domain-inference.ts; drop remaining .nycrc source excludes Mock @langchain/anthropic to cover the roster/inference logic + parse-error branches (now 100%). Remove domain-inference + open-review-pr excludes; .nycrc matches main. Gate 96.5/85.96/97.3/96.9. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
npm run validate-contextscript that validatesagent-memory/context files againstschema.jsonschema.jsonfrontmatter definition withTEMPLATE.mdfields (id,category,domain,issueNumber,issueUrl,title,lastUpdated,summary,services,techStack)Context
Issue #11 had 4/5 acceptance criteria already met via #1, #20, and #72. This PR addresses the remaining two:
npm run validate-contextcommand)All 10 existing context files in
agent-memory/domains/forms-and-reports/issues/pass validation.Test plan
npm run validate-contextpasses all 10 existing context filesnpm run validate-context -- path/to/invalid-file.mdreports errors for malformed filesnpm testpasses all 154 tests (130 existing + 24 new)npm run lintshows no new errors