feat(#126): introduce Langfuse observability for the memory pipeline by alexosugo · Pull Request #127 · medic/cht-agent

alexosugo · 2026-06-24T07:24:03Z

Summary

Closes #126
Closes #103

Introduces end-to-end LLM observability for the memory distillation pipeline using Langfuse.

Note on #103 scope: #103 asked for a self-hosted Langfuse deployment via Docker Compose. This PR ships Langfuse Cloud instead — self-hosting is deferred until the agent platform has matured enough to justify the operational overhead of running our own instance. Trace capture, span structure, and the LangChain integration itself fully satisfy the eval-foundation goal of #103; only the hosting model differs from what was originally scoped.

src/observability/index.ts — shared module: lazy singleton Langfuse client (getLangfuse()) plus a single startTrace() primitive that returns { trace, handler } — a trace and the LangChain callback handler rooted on it. Replaces the earlier separate makeLangfuseHandler/createTrace/flushLangfuse helpers. All calls are no-ops when LANGFUSE_ENABLED=false.
src/types/pipeline.ts — FilterOptions and DistillOptions gain an optional langfuseHandler?: BaseCallbackHandler field.
src/scripts/filter.ts — threads the handler to chain.invoke({ callbacks }) inside llmTriage.
src/scripts/distiller.ts — threads the handler to chain.invoke({ callbacks }) inside llmDistill.
src/scripts/run-pipeline.ts — processSinglePR creates one Langfuse trace per PR via startTrace() (with a scrape span and a distill-outcome score), sets trace.update({ output }), and flushes via getLangfuse().flushAsync(). All traces from a single runPipeline call share a session UUID.
.github/workflows/unit_tests.yml — sets LANGFUSE_ENABLED=false so CI tests never contact Langfuse.
.github/workflows/run-pipeline.yml — adds LANGFUSE_PUBLIC_KEY, LANGFUSE_SECRET_KEY, and LANGFUSE_BASE_URL from repository secrets/vars.
docs/observability.md — setup guide, trace structure, naming conventions, and a step-by-step recipe for instrumenting new agents/workflows, plus a "Future Work" section (prompt versioning, dashboards, alerting, eval datasets, agent instrumentation).
.env.example — documents the four Langfuse variables (LANGFUSE_PUBLIC_KEY, LANGFUSE_SECRET_KEY, LANGFUSE_BASE_URL, LANGFUSE_ENABLED).

Trace shape (per PR)

memory-pipeline-pr   (input: { prNum, repo, url }; tags: [memory-pipeline, <repo>]; session: <run UUID>)
├── span: scrape          input: { prNum, repo }  output: { fileCount }
├── generation: filter    ← LangChain callback auto-captures model + tokens + latency
└── generation: distill   ← LangChain callback auto-captures model + tokens + latency
    score: distill-outcome           1 = written, 0 = flag-for-human

The trace id is generated by Langfuse per run (not derived from the PR number), so reprocessing the same PR produces a distinct trace rather than mutating an earlier run's session. PR identity lives in input/tags/metadata so it stays filterable.

Design notes

Lazy singleton: the Langfuse client is created on first call (not at module load time) so dotenv.config() runs before the constructor reads process.env.
LANGFUSE_ENABLED=false: startTrace()'s handler is still constructed but no-ops; callers use handler ? [handler] : undefined to skip callbacks entirely.
Env var naming: LANGFUSE_HOST was renamed to LANGFUSE_BASE_URL across source, docs, workflow, and .env.example for clarity (matches the Langfuse client constructor option name).
Injection point preserved: opts.triageFn / opts.distillFn in tests still bypass the real LLM and the handler, keeping test isolation intact.
Scoring today: distill-outcome is the only score currently emitted, set directly in code (trace.score(...) in run-pipeline.ts) — not an LLM-judge or human annotation. It only fires for PRs that reach the distill stage; filtered-out PRs get an unscored trace with decision: 'skip' in the output.

Test plan

npm run build — tsc clean
npm run lint — eslint clean
LANGFUSE_ENABLED=false npm run test:coverage — 416 tests pass, all coverage thresholds met (exit 0)
Ran npm run run-pipeline -- --pr <cht-core-pr> locally with real Langfuse keys and confirmed a trace appears in the Langfuse Cloud dashboard with scrape span, LLM generations, and the distill-outcome score.
Add LANGFUSE_PUBLIC_KEY and LANGFUSE_SECRET_KEY as repository secrets, and LANGFUSE_BASE_URL as a repository variable before the next scheduled pipeline run.

sugat009

Requesting changes; findings are in the inline comments below.

sugat009 · 2026-07-13T15:04:39Z

    "gray-matter": "^4.0.3",
    "js-yaml": "^4.1.1",
+    "langfuse": "^3.38.20",
+    "langfuse-langchain": "^3.38.20",


issue (blocking): langfuse-langchain@3.38.20 peer-requires langchain >=0.0.157 <0.4.0 (i.e. @langchain/core 0.3.x), but main is now on @langchain/core ^1.2.1. No published langfuse-langchain supports core v1, so a fresh npm ci after rebasing onto main will ERESOLVE. CI is green here only because this branch still pins the 0.3.x stack. Needs a resolution: manual langfuse instrumentation instead of the langfuse-langchain handler, pin langchain back (regresses main), or await a v1-compatible release.

sugat009 · 2026-07-13T15:04:39Z

@@ -0,0 +1,8 @@
+#!/bin/sh
+# roborev post-commit hook v4 - auto-reviews every commit


issue (blocking, accidental): this and .husky/post-rewrite run local roborev tooling (hardcoded /opt/homebrew/bin/roborev), and .gitignore gains /.roborev/ + .roborev.toml. husky is wired via postinstall: husky install, so these run on every contributor's commit/rewrite. Unrelated to #126 — please drop them from the PR.

sugat009 · 2026-07-13T15:04:39Z

 fi
+
+# agent-memory/_skipped.ndjson is a local runtime log, never real content — keep it empty in git.
+skip_log="agent-memory/_skipped.ndjson"


issue (scope): this skip-log emptier, together with the agent-memory/_pending/**/*.md gitignore rule and the emptied _skipped.ndjson, is runtime-log hygiene. Reasonable on its own but outside the #126 observability scope and unmentioned in the description; consider splitting it out.

sugat009 · 2026-07-13T15:04:39Z


-  const result = await chain.invoke(prompt) as TriageOutput;
+  const callbacks = handler ? [handler] : undefined;
+  const result = await chain.invoke(prompt, { callbacks }) as TriageOutput;


issue: under LLM_PROVIDER=claude-cli, chain is createStructuredCliChain, whose invoke(prompt) ignores this { callbacks } argument, so the Langfuse handler never fires and no token/cost is captured on the claude -p path (only the trace, scrape span, and score). Token/cost — the PR's headline goal — is captured only via the LangChain generation callback, which works on the OpenRouter/Anthropic path the scheduled workflow uses but silently not on the CLI path. docs/observability.md's unconditional "auto-captures model + tokens" overstates the CLI case.

sugat009 · 2026-07-13T15:04:39Z

  }
+
+  trace.update({ output });
+  await getLangfuse().flushAsync();


issue (minor): no try/finally around scrape/filter/distill, so if any throws, trace.update()/score()/flushAsync() are skipped and the failed PR leaves an incomplete, unflushed trace with no error recorded. Also this flushAsync() runs per-PR on the shared singleton and runPipeline has no final flush after Promise.all, so a last-in-flight PR that errors before its own flush can lose buffered events. A try/finally that records the error and flushes closes both.

sugat009 · 2026-07-13T15:04:39Z

+```
+memory-pipeline-pr   (input: { prNum, repo, url }; tags: [memory-pipeline, <repo>])
+├── span: scrape          (no LLM — input: prNum, repo; output: fileCount)
+├── generation: filter    (LangChain callback — model + tokens auto-captured)


issue (minor, doc drift): the diagram labels these generation: filter / distill, but the code sets withConfig({ runName: 'triage-classify' }) / 'distill-draft', so the actual Langfuse generation names differ from the docs.

sugat009 · 2026-07-13T15:04:39Z

 # DEEPWIKI_MCP_TIMEOUT=30000
+
+# Langfuse LLM observability (https://cloud.langfuse.com)
+# Set LANGFUSE_ENABLED=false to disable tracing locally (default: enabled when keys are present)


issue (minor): this says tracing is enabled "when keys are present", but the code enables whenever LANGFUSE_ENABLED !== 'false' regardless of keys (the SDK then self-disables if keys are missing). The stated rule is inaccurate.

- Add src/observability/index.ts — lazy singleton Langfuse client, makeLangfuseHandler factory, createTrace and flushLangfuse helpers - Thread optional langfuseHandler through FilterOptions and DistillOptions - filter.ts / distiller.ts pass the handler to chain.invoke callbacks - run-pipeline.ts creates one trace per PR with scrape span and distill-outcome score; groups all PR traces under a shared session UUID - Disable tracing in CI (LANGFUSE_ENABLED=false in unit_tests.yml) - Add Langfuse secrets to run-pipeline.yml production env - Add docs/observability.md covering setup, naming conventions, and how to instrument new agents or workflows - Update .env.example with Langfuse variables

- makeLangfuseHandler: drop LANGFUSE_ENABLED guard (SDK no-ops when enabled=false); return type narrows from CallbackHandler|undefined to CallbackHandler - export getLangfuse(); delete flushLangfuse wrapper; callers call getLangfuse().flushAsync() directly - delete resetLangfuseClient (unreachable from production code; tests mock the whole module) - remove ?? traceId fallback in processSinglePR; pass sessionId directly to both makeLangfuseHandler and createTrace

…ce() Collapse makeLangfuseHandler()/createTrace() into a single startTrace() returning { trace, handler }, so callers no longer juggle two related objects built from the same trace id. Let the Langfuse SDK generate trace ids instead of handcrafting them from PR number — the old scheme could silently corrupt session groupings when overlapping --since windows reprocessed the same PR. Update run-pipeline.ts wiring, docs, and add a direct unit test for the disabled no-op path.

This is a runtime audit log appended to by local pipeline/test runs, not a file with content meant to be checked in. Truncate the stale entries that had accumulated in the repo.

Auto-empties and re-stages the file if it's non-empty at commit time, so local pipeline/test runs appending to it never leak into git history.

Match the variable name actually used in .env across the module, docs, and CI workflow.

Rebase conflict resolution introduced a bare optional sessionId param after two defaulted params; give it an explicit default instead.

Override withStructuredOutput() chain names via withConfig({ runName }) so triage/distill spans show as triage-classify/distill-draft instead of the generic RunnableSequence in Langfuse traces. Also collapse processSinglePR's 5 positional params into an options object to satisfy SonarQube S107 (max 4 params).

alexosugo · 2026-08-12T15:41:12Z

@sugat009 All issues have been addressed. This is ready for another round of review.

sugat009

Thanks for the rework. The dependency fix is better than a version pin: langfuse-langchain is gone from package.json, the lockfile and every import, and langfuse@3.38.20 declares no peer dependencies, so the @langchain/core 1.x conflict cannot recur.

Six of my seven July items verify fixed at 16cbd1a: roborev hooks and .gitignore byte-identical to main, 766f008 reverted the _skipped.ndjson and pre-commit scope creep, try/finally wraps the pipeline with trace.update on error, all five docs observation names match the code, and the LANGFUSE_BASE_URL rename has zero stragglers.

Two blocking items. Removing the CallbackHandler removed the token source, so the PR ships no cost data, which is #126's first goal. Roughly six small changes, not a redesign. The second only bites when Langfuse is unreachable.

sugat009 · 2026-08-13T08:55:12Z

+  const generation = trace?.generation(opts);
+  try {
+    const output = await invoke();
+    generation?.end({ output });


issue (blocking): observeGeneration ends generations with { output } and never sends usage, so Langfuse shows no tokens or cost on either path. #126 opens with "Cost opacity", so this is the acceptance criterion. Grepping the diff for usage|totalTokens|input_tokens|promptTokens hits only the docs sentence admitting it.

Not blocked, just unwired: langfuse-core's UpdateGenerationBody accepts usage/usageDetails/costDetails on end(), and @langchain/core supports withStructuredOutput(schema, { includeRaw: true }) returning { raw, parsed } with raw.usage_metadata. Four chain builders, two call sites, the test doubles.

sugat009 · 2026-08-13T08:55:12Z

+  } catch (err) {
+    trace.update({ output: { error: errorMessage(err) } });
+    throw err;
+  } finally {


issue (blocking): the finally awaits flushAsync() per PR at SDK defaults: requestTimeout 5000ms, fetchRetryCount 3, fetchRetryDelay 3000ms, and retryCheck matches HTTP errors too, so a rotated key retries like a timeout. About 29s per PR: 7 minutes lost on a 15-PR nightly, 48 on a --last 100 backfill, plus ~60 SDK error lines.

Separately, flush() drains only flushAt (15) events and flushAsync never awaits pendingIngestionPromises, while reportOutcome calls process.exit. At --concurrency > 1 that kills an in-flight POST, losing events from the failed runs you most want.

Use one await getLangfuse().shutdownAsync() at the end of runPipeline instead of the per-PR flush; it clears the timer, flushes, and awaits pending promises. Add fetchRetryCount: 1 and requestTimeout: 3000.

sugat009 · 2026-08-13T08:55:12Z

+
+    const span = trace.span({ name: 'step', input: { x: 1 } });
+    span.end({ output: { y: 2 } });
+    trace.score({ name: 'outcome', value: 1 });


issue (non-blocking): the specs cannot detect an observability regression. The one test touching the real module sets LANGFUSE_ENABLED=false itself, and _client is cached with no reset seam, so an enabled client is unreachable in-process. run-pipeline.spec substitutes a spy-free noop.

Delete { langfuseTrace: trace } at run-pipeline.ts:314 and :337, reintroducing the uninstrumented-CLI bug this PR fixes, and every test passes. Delete the finally flush: still passes. Latent: the noopTrace double lacks generation, so the first workflow following the docs' step 3 throws.

sugat009 · 2026-08-13T08:55:12Z

  return _triageChain;
 }

+function getTriageModel(): string {


issue (non-blocking): the CLI path labels the model claude-cli and discards the per-call USD cost the adapter already parses. Langfuse cannot price a model name that does not exist, and real cost data is dropped, on the one path where it is available today.

sugat009 · 2026-08-13T08:55:13Z

+    const output = await invoke();
+    generation?.end({ output });
+    return output;
+  } catch (err) {


suggestion (non-blocking): failed generations and traces record the error in output but never set level: 'ERROR' or statusMessage, so Langfuse error views and future alerting see zero failures. langfuse-core accepts both on end()/update().

sugat009 · 2026-08-13T08:55:13Z

+  opts: { force: boolean; tag: string; trace: ReturnType<typeof startTrace>['trace'] }
+): Promise<void> {
+  const { force, tag, trace } = opts;
+  const scrapeSpan = trace.span({ name: 'scrape', input: { prNum, repo } });


nitpick: the scrape span is never ended when scrapePR throws, orphaning an observation on every failed PR.

sugat009 · 2026-08-13T08:55:13Z

    });
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    return (llm as any).withStructuredOutput(triageSchema);
+    return (llm as any).withStructuredOutput(triageSchema).withConfig({ runName: 'triage-classify' });


nitpick: withConfig({ runName }) is dead now that names come from observeGeneration, and it forced shims into three test doubles. Removing it simplifies the specs.

sugat009 · 2026-08-13T08:55:13Z

+   });
+   ```
+
+3. Wrap each model call in a generation. This works for both LangChain API calls


suggestion (non-blocking): the recipe never mentions observeGeneration, the actual entry point, and its test stub omits generation, contradicting step 3. The token caveat also needs rescoping: the API path has no usage either.

github-project-automation Bot added this to CHT Multi-Agent System (cht-agent) Jun 24, 2026

github-project-automation Bot moved this to Todo in CHT Multi-Agent System (cht-agent) Jun 24, 2026

alexosugo moved this from Todo to In Progress in CHT Multi-Agent System (cht-agent) Jun 24, 2026

alexosugo force-pushed the main branch from f4af7fb to f1de413 Compare June 24, 2026 11:15

alexosugo self-assigned this Jun 24, 2026

alexosugo force-pushed the feat/126-langfuse-observability branch 2 times, most recently from babdb7b to 42726c5 Compare July 1, 2026 15:32

alexosugo requested review from Hareet and sugat009 July 1, 2026 15:35

sugat009 mentioned this pull request Jul 3, 2026

feat(#62): implement code generation layer #86

Merged

7 tasks

sugat009 moved this from In Progress to In Review in CHT Multi-Agent System (cht-agent) Jul 3, 2026

sugat009 requested changes Jul 13, 2026

View reviewed changes

sugat009 mentioned this pull request Jul 21, 2026

chore(memory): promote strong-fit forms-and-reports drafts from memory-pipeline for review #122

Open

alexosugo added 9 commits August 12, 2026 18:26

chore: keep agent-memory/_skipped.ndjson empty in git

ba6ef06

This is a runtime audit log appended to by local pipeline/test runs, not a file with content meant to be checked in. Truncate the stale entries that had accumulated in the repo.

chore: pre-commit hook to keep agent-memory/_skipped.ndjson empty

80d7212

Auto-empties and re-stages the file if it's non-empty at commit time, so local pipeline/test runs appending to it never leak into git history.

chore(#126): rename LANGFUSE_HOST env var to LANGFUSE_BASE_URL

6bc4b31

Match the variable name actually used in .env across the module, docs, and CI workflow.

fix: satisfy default-param-last lint rule in processSinglePR

d8a6abd

Rebase conflict resolution introduced a bare optional sessionId param after two defaulted params; give it an explicit default instead.

fix: address Langfuse observability review

766f008

alexosugo force-pushed the feat/126-langfuse-observability branch from 39a0aa5 to 766f008 Compare August 12, 2026 15:31

alexosugo added 2 commits August 12, 2026 18:37

fix: reduce observability complexity

41e5e2a

fix: satisfy SonarQube parameter limit

16cbd1a

alexosugo requested a review from sugat009 August 12, 2026 15:40

sugat009 requested changes Aug 13, 2026

View reviewed changes

sugat009 mentioned this pull request Aug 13, 2026

fix(#136): memory pipeline hardening + issue relinking (#135) #138

Open

3 tasks

		@@ -0,0 +1,8 @@
		#!/bin/sh
		# roborev post-commit hook v4 - auto-reviews every commit

Uh oh!

Conversation

alexosugo commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Trace shape (per PR)

Design notes

Test plan

Uh oh!

sugat009 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexosugo commented Aug 12, 2026

Uh oh!

sugat009 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alexosugo commented Jun 24, 2026 •

edited

Loading