chore(memory): promote strong-fit forms-and-reports drafts from memory-pipeline for review#122
Open
Hareet wants to merge 50 commits into
Open
chore(memory): promote strong-fit forms-and-reports drafts from memory-pipeline for review#122Hareet wants to merge 50 commits into
Hareet wants to merge 50 commits into
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.
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>
…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>
… 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>
…ructure to VALID_DOMAINS Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ft 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>
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>
…n'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>
…ons, re-export) Shared linear JSON extractor; CC refactors + justified NOSONAR; more Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…R 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>
…rameters) 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>
Hareet
force-pushed
the
seeding-claude-cli-v2
branch
from
551acb2 to
e90aef1
Compare
Member
Author
|
Needs to be rebased with main after #119 is merged |
Member
|
Heads up, this one currently has merge conflicts with its base and will need a rebase before it can merge. |
Hareet
force-pushed
the
seeding-claude-cli-v2
branch
from
1979c05 to
6a2742a
Compare
This was referenced Jul 1, 2026
Open
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
3 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.
Promotes 47 strong-fit
forms-and-reportsdrafts fromagent-memory/_pending/intoagent-memory/domains/forms-and-reports/issues/for squad content review.Categories: feature (21), bug (20), improvement (6)
Themes: Enketo widgets & XPath extensions, cht-datasource report create/update, SMS parser, form validation/submission hardening.
All 47 carry
domainFit: strong+ a## Domain Rationalesection. 5 weak-fit drafts deferred for later.seeding-claude-cli-v2(feat(#108): seeding pipeline - CLI provider, domain-rationale, infrastructure domain, concurrency #119) — retarget tomainafter the schema lands.validate-schema: passing, 0 failures.