mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 08:53:22 +00:00
fix(conversation-parser): add markdown-heading turn pattern (## User / ## Assistant) (#4005)
* fix(facts): make transcript pages facts-extraction eligible
`gbrain extract-conversation-facts`'s ALLOWED_TYPES allowlist omitted the
`transcript` page type, so gbrain's own nightly transcript-ingest pages
were silently skipped by both the CLI `--types` validation and the
`cycle.conversation_facts_backfill.types` config filter. Even with the
type allowed, the built-in conversation-parser had no pattern for the
`## User` / `## Assistant` markdown-heading turn shape that transcript
ingest writes into `compiled_truth`, so parsing would still yield 0
segments.
This PR makes an explicit decision: transcript pages ARE now
facts-extraction eligible. That is a real behavioral change (a new,
potentially large corpus starts flowing through the extraction +
segment-cost path), not a no-op bugfix — flagging it plainly rather than
padding out the change as narrower than it is.
Changes:
- `src/commands/extract-conversation-facts.ts`: add `'transcript'` to
`ALLOWED_TYPES` / `ALLOWED_TYPE_ALIASES` (the single source of truth
for this allowlist).
- `src/core/conversation-parser/builtins.ts`: add the `markdown-heading-turn`
builtin pattern recognizing heading-only `## User` / `## Assistant` /
`## Human` / `## System` lines as turn openers, with D5 continuation-line
body absorption. `quick_reject` is deliberately scoped to the role-prefix
(not a bare `#{2,3}` heading check) so a message body that happens to
paste unrelated markdown headings doesn't starve the D18 scorer's
anchor-candidate ratio.
- `src/commands/jobs.ts`, `src/commands/doctor.ts` (x2 checks),
`src/commands/sources.ts`: these each carried their own hand-copied
literal of the same allowed-types list (background-job type filter,
`conversation_facts_backlog` doctor check, `conversation_format_coverage`
doctor check, `facts_backfill_estimate`). Switched each to import
`ALLOWED_TYPES` from the command module instead of re-listing it, so this
class of drift (a type added in one place, silently excluded everywhere
else) can't recur.
- `docs/architecture/KEY_FILES.md`: updated the two stale mentions (pattern
count 17→18, allowlist list) to current-state per this repo's own
reference-doc convention.
Known limitation (not fixed here, scope-bounded intentionally): parsing is
context-free, same as every other multi-line builtin in this registry — a
message body that contains a literal `## User` line (e.g. someone pasting
a markdown transcript excerpt into their own message) would be read as a
turn boundary. This is a pre-existing property of the whole parser
(`applyPattern`'s per-line scan has no fence-awareness), not something
this PR introduces or could fix without a much larger, separate change to
the shared orchestrator affecting all 18 patterns. Flagging it here rather
than silently shipping the same limitation as the other 17 builtins.
Tests: 4 new tests (2 in test/extract-conversation-facts.test.ts, 2 in
test/conversation-parser/parse.test.ts) covering the allowlist, the new
pattern's positive match + continuation absorption, and that ordinary
`## Summary`-style headings are correctly rejected. Full targeted suite
(conversation-parser + facts-extraction + doctor backlog + build-llms
freshness): 263 pass / 0 fail. typecheck clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0126D3zLWL5RE3CVxnPANiiU
* fix(facts): read the type allowlist from core, not the command module
CI caught this: the known-flags registry drifted for doctor, sources, and
repos. The obvious remedy the guard prints -- regenerate and commit -- would
have been a regression, so this takes the other route.
The generator walks one level of a command module's relative imports and
harvests every flag-shaped string it finds, help text included, and is
deliberately over-inclusive. Importing extract-conversation-facts.ts just to
read ALLOWED_TYPES therefore spliced that command's entire flag vocabulary
(--types, --sleep, --slug, --segment-limit, --override-disabled, ...) into
the allowlists of three commands that implement none of it: `gbrain doctor
--types foo` would have passed validation and been silently ignored. That is
the exact defect class #2185 exists to close.
(jobs.ts is unaffected: it already imported the command module on one line
for runExtractConversationFactsCore, so those flags were already in its
registry entry before this branch.)
ALLOWED_TYPES + ALLOWED_TYPE_ALIASES now live in
src/core/conversation-facts-types.ts, a constants-only module with no CLI
text to harvest. extract-conversation-facts.ts re-exports both so its
existing importers are unchanged.
Verified: registry regenerates to zero drift (was doctor/repos/sources),
cli-flag-validation 24 pass, typecheck clean, 287 pass across the touched
areas. Confirmed against a clean upstream/master worktree that the drift was
introduced by this branch and is not pre-existing.
* fix(conversation-parser): reduce to the parser pattern only
Withdraws the `transcript` allowlist half of this branch. The premise was
wrong: `transcript` is not an upstream page type. `ALL_PAGE_TYPES` does not
contain it, `gbrain-base.yaml` declares `conversation` for "long-running
chat/transcript pages" and marks it `extractable: true` precisely so
extract-conversation-facts walks it, and `gbrain-base-v2.yaml` lists
`transcript` as an alias of `source` (a media primitive). Pages typed
`transcript` are a convention of my own ingest pipeline, not something
upstream produces — the fix for that belongs on my side, by emitting
`conversation`.
That takes the four call-site de-duplications with it (they existed only to
keep the allowlist in sync), and with them the flag-registry drift: no
imports are added, so the registry regenerates to zero drift with no
constants module needed.
What remains is the half that stands on its own: a `conversation` page whose
body uses `## User` / `## Assistant` headings matches none of the 17 builtins
and parses to 0 segments. `markdown-heading-turn` is an 18th pattern in the
same shape as the iMessage/Circleback additions before it.
Verified: typecheck clean, 181 pass / 0 fail across the parser, extraction,
flag-registry and llms-freshness suites, registry drift zero.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -276,6 +276,33 @@ describe('parseConversation — disabledBuiltinIds', () => {
|
||||
// Multi-line continuation (D5)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('parseConversation — markdown-heading-turn (gbrain transcript ingest)', () => {
|
||||
test('parses ## User / ## Assistant heading-only turns with continuation body', () => {
|
||||
const body = [
|
||||
'## User',
|
||||
'What is the capital of France?',
|
||||
'## Assistant',
|
||||
'The capital of France is Paris.',
|
||||
'It is also its largest city.',
|
||||
].join('\n');
|
||||
const r = parseConversation(body, { fallbackDate: '2026-08-11' });
|
||||
expect(r.matched_pattern_id).toBe('markdown-heading-turn');
|
||||
expect(r.messages).toHaveLength(2);
|
||||
expect(r.messages[0].speaker).toBe('User');
|
||||
expect(r.messages[0].text).toBe('What is the capital of France?');
|
||||
expect(r.messages[1].speaker).toBe('Assistant');
|
||||
expect(r.messages[1].text).toBe(
|
||||
'The capital of France is Paris.\nIt is also its largest city.',
|
||||
);
|
||||
});
|
||||
|
||||
test('does not mistake an ordinary ## Summary heading for a turn', () => {
|
||||
const body = ['## Summary', 'This is not a speaker turn.'].join('\n');
|
||||
const r = parseConversation(body, { fallbackDate: '2026-08-11' });
|
||||
expect(r.matched_pattern_id).not.toBe('markdown-heading-turn');
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseConversation — multi-line continuation (D5)', () => {
|
||||
test('iMessage continuation absorbs orphan lines', () => {
|
||||
const body = [
|
||||
|
||||
@@ -146,6 +146,24 @@ test('conversation-facts allowlist includes native iMessage page types (#2756)',
|
||||
expect(ALLOWED_TYPES).toContain('imessage-daily');
|
||||
});
|
||||
|
||||
test('parses a markdown-heading turn body (## User / ## Assistant)', () => {
|
||||
const body = [
|
||||
'## User',
|
||||
'What is the capital of France?',
|
||||
'## Assistant',
|
||||
'The capital of France is Paris.',
|
||||
'It is also its largest city.',
|
||||
].join('\n');
|
||||
const msgs = parseConversationMessages(body, { fallbackDate: '2026-08-11' });
|
||||
expect(msgs).toHaveLength(2);
|
||||
expect(msgs[0].speaker).toBe('User');
|
||||
expect(msgs[0].text).toBe('What is the capital of France?');
|
||||
expect(msgs[1].speaker).toBe('Assistant');
|
||||
expect(msgs[1].text).toBe(
|
||||
'The capital of France is Paris.\nIt is also its largest city.',
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// splitIntoSegments — PR's 5 cases verbatim plus tuning regression.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user