Skip to content

chore(memory): promote strong-fit messaging drafts from memory-pipeline for review#120

Open
Hareet wants to merge 50 commits into
mainfrom
memory/promote-messaging
Open

chore(memory): promote strong-fit messaging drafts from memory-pipeline for review#120
Hareet wants to merge 50 commits into
mainfrom
memory/promote-messaging

Conversation

@Hareet

@Hareet Hareet commented Jun 24, 2026

Copy link
Copy Markdown
Member

Promotes 17 strong-fit messaging drafts from agent-memory/_pending/ into agent-memory/domains/messaging/issues/ for squad content review.

Categories: bug (10), feature (7)
Themes: SMS gateway integrations (Africa's Talking, Nepal centralized gateway, RapidPro), scheduled-task/message-state handling, mustache phone helpers.

All 17 carry domainFit: strong + a ## Domain Rationale section. Weak-fit messaging drafts: 0 (none deferred).

Merge order: #1 of 9 (messaging → infrastructure → forms-and-reports → tasks-and-targets → authentication → contacts → interoperability → configuration → data-sync).

alexosugo added 30 commits May 12, 2026 19:19
- 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.
… 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)
alexosugo and others added 19 commits June 16, 2026 15:36
- 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>
…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>
…rlaps #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>
…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

Hareet commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

@sugat009 Needs to be rebased with main after #119 is merged

@sugat009

sugat009 commented Jun 26, 2026

Copy link
Copy Markdown
Member

Heads up, this one currently has merge conflicts with its base and will need a rebase before it can merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants