feat(#108): memory distillation pipeline — scraper, filter, distiller, CLI, and CI workflow#128
Merged
Merged
Conversation
- 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
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
- 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)
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.
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%
…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).
'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.
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
…ts, 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
Adds distillPR call after a distill decision so smoke-test validates the full scrape → filter → distill pipeline in one run.
… missing frontmatter
… 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.
- 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
…d 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 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+)
- 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)
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)
- 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()
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.
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.
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.
- 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)
- 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
- 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).
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.
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.
- 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).
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.
alexosugo
added
Type: Feature
3 tasks
Hareet
approved these changes
Jun 24, 2026
Hareet
left a comment
Hareet
left a comment
Member
There was a problem hiding this comment.
I think this is the same code/PR that was already approved. I'm just approving based on that.
github-project-automation
Bot
moved this from Todo
to Done
in CHT Multi-Agent System (cht-agent)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR wires up the automated memory pipeline for
agent-memory/— instead of hand-writing knowledge entries, we scrape mergedcht-corePRs, decide whether they're worth capturing, and use an LLM to draft a structured note. A human still reviews before anything lands indomains/.Each stage is independently testable and runnable from one CLI or the CI workflow.
How each stage works
scraper.ts) — pulls everything about a PR via theghCLI: title, labels, author, file list, full diff, linked issues, reviewer comments. No LLM involved — just data assembly.filter.ts) — keep vs. skip in two passes. First: fast deterministic rules (skip chore/test/dependency-only PRs; keep code changes with a linked issue). Then an LLM judges anything ambiguous. Every skip goes to_skipped.ndjsonwith a reason so the rules can be audited and tuned.distiller.ts) — sends kept PRs to an LLM and writes a structured draft toagent-memory/_pending/<domain>/<slug>.mdwith provenance frontmatter (source_pr,source_sha,confidence, etc.). Drafts that fail schema validation go to_skipped.ndjsoninstead.Running it
run-pipelineCLI (run-pipeline.ts) — wires all three stages. Flags:--pr <n>(single PR),--since <h>(recent merges, default 24h),--repo <owner/name>(defaultmedic/cht-core). Exits non-zero if any PR fails..github/workflows/run-pipeline.yml) — runs daily at 06:00 UTC over the last 24h, with a manualworkflow_dispatchoption. Commits new drafts and_skipped.ndjsonupdates with[skip ci].open-review-pr(open-review-pr.ts) — the human side: opens each_pending/draft in your editor, validates on save, moves approved drafts toagent-memory/domains/.Config
.env(see.env.example):OPENROUTER_API_KEY,TRIAGE_MODEL,DISTILL_MODEL,GH_TOKEN.Also in this PR
schema-utils.ts(AJV validator + frontmatter helpers),src/types/pipeline.ts(pipeline types),src/constants/,agent-memory/schema.json+TEMPLATE.md.scraper,filter,distiller,open-review-pr) —ghmocking, every filter branch, LLM happy/invalid paths, the review loop. 415 passing.[^/]*instead of.*). The S4036 PATH hotspots onexecFileSync('gh', ...)calls are accepted as-is — no shell is used, args are arrays, they only run in controlled CI, and hardcoding an absolute path would break portability.Review items addressed
schema.jsonandschema-utils.tsso both PRs share one contract. Distiller emits camelCase frontmatter includingissueNumber,issueUrl,services, andtechStack.distillPRserializes withjs-yamldump()and AJV-validates every draft before writing. Invalid drafts go to_skipped.ndjsonasflag-for-human.os.tmpdir()insmoke-test.tsreplaced withfs.mkdtempSync(random name, 0700 perms).--paginate --slurpand flattened — no longer corrupts bodies with] [in them.--sinceand--prrejected with a clear error.user: nullin reviews falls back toghost.source_prURL. Resolves to/pull/<n>, not a dead anchor./**inconstants/index.tsrestored — was causing 30+ TS errors in CI.Follow-up issues
claude ppopulates the memory pipeline #124 — CLI factory soclaude pruns the pipeline from the Claude Code promptTest plan
npm test— all 415 tests passnpm run smoke-test— full pipeline against PRs 11057, 11022, 11077; expect at least one distill and one skipnpm run run-pipeline -- --pr 11057— produces a valid draft in_pending/npm run run-pipeline -- --since 48— runs clean;_skipped.ndjsongrows for skipsworkflow_dispatchwithpr_number=11057— workflow commits a draft back to the branchnpm run open-review-pr— opens, validates, and moves a draft to the right domainCloses #108 and lays the foundation for #101