Phase 3 — The data-model guide

docs/mcp-extension/plan.md

@@ -172,7 +172,14 @@ balance_report.
 
 ---
 
-## Phase 3 — The data-model guide (§8)
+## Phase 3 — The data-model guide (§8) — ✅ DONE
+
+_Implemented as `src/tools/data-model-guide.ts`, registered second (right after membership
+discovery). 480 tests pass; codegen, lint, typecheck and build clean. Two deviations from the plan
+below, both deliberate: the guide is ~5.8KB rather than ~4KB, and the card-settlement claim was
+corrected against the code — `CreditcardBankCharge` **does** link the settlement transaction to the
+card rows it covers (`creditCardTransactions`, `validCreditCardAmount`), so the guide points at that
+grouping instead of repeating the feedback's "no link between them"._
 
 The feedback says a static doc "would have saved the entire reverse-engineering phase." The MCP
 handler advertises only `capabilities: { tools: { listChanged: false } }`

packages/mcp-server/README.md

@@ -18,8 +18,8 @@ Phase 1 (read-only) is feature-complete. The server provides: strict startup env
 transport with `/health`, `/metrics`, the OAuth protected-resource metadata endpoint, and the MCP
 route (`POST /mcp`, JSON-RPC 2.0) with graceful shutdown; Auth0 bearer-token verification; identity
 mapping to an internal user + business-membership context with memberships resolved from the
-Accounter GraphQL server; a curated registry of fifteen read-only tools
-(`accounter_list_business_memberships`, `accounter_list_accounts`,
+Accounter GraphQL server; a curated registry of sixteen read-only tools
+(`accounter_list_business_memberships`, `accounter_data_model_guide`, `accounter_list_accounts`,
 `accounter_income_expense_summary`, `accounter_profit_and_loss`, `accounter_vat_report`,
 `accounter_counterparty_totals`, `accounter_search_charges`, `accounter_get_charges`,
 `accounter_get_transactions`, `accounter_get_documents`, `accounter_ledger_records`,
@@ -78,6 +78,15 @@ scope, because it _is_ the scope.
   businesses last. Pure: memberships are already on the auth context, so it makes no upstream call.
   A caller with no memberships gets an empty list, not an error. This is the scope-discovery entry
   point; to browse the full business directory use `accounter_list_businesses`.
+- **`accounter_data_model_guide`** — a static markdown explanation of how accounts, charges,
+  documents, currency, and date filters relate, and of the traps that make naive aggregation wrong.
+  Pure (no upstream call) and **needs no business scope**, so it can be read cold, before a business
+  is chosen — which is the point: the connector feedback identified this missing context as the
+  single most expensive gap, since an agent otherwise reverse-engineers the model from Hebrew bank
+  descriptions. It is a tool rather than an MCP resource because the transport advertises only
+  `tools` capabilities, and `tools/list` is where a model actually looks. `data-model-guide.test.ts`
+  pins its content against the code — every tool it names must be registered, and every charge and
+  account type must be documented — so a rename cannot leave it silently stale.
 
 The **report tools** are registered ahead of the row-level ones on purpose: a model asked "how much
 did we make this year" should assemble and explain a report, not derive one from raw bank rows. All

packages/mcp-server/docs/todo.md

@@ -10,7 +10,7 @@ Last consolidated 2026-08-03, after I1/I2/I4 (#4103–#4105) merged into `main`.
 owner-scoping work (#4089–#4097) is also in.
 
 **Status of the connector today:** working end-to-end against Claude Desktop with a pre-registered
-Auth0 client, read-only, fifteen curated tools, business scope enforced by RLS upstream. Not
+Auth0 client, read-only, sixteen curated tools, business scope enforced by RLS upstream. Not
 publishable — see B1.
 
 ---

packages/mcp-server/src/tools/__tests__/data-model-guide.test.ts

@@ -0,0 +1,152 @@
+import { describe, expect, it, vi } from 'vitest';
+import { buildAuthContext, type McpAuthContext } from '../../auth/identity.js';
+import type { AuthPrincipal } from '../../auth/token.js';
+import { UpstreamGraphQLClient } from '../../upstream/graphql-client.js';
+import {
+  DATA_MODEL_GUIDE,
+  DATA_MODEL_GUIDE_TOOL_NAME,
+  dataModelGuideTool,
+} from '../data-model-guide.js';
+import { KNOWN_CHARGE_TYPENAMES, chargeTypeFromTypename } from '../entity-shapes.js';
+import { executeRegisteredTool } from '../execute.js';
+import { toolRegistry } from '../registry-instance.js';
+
+/**
+ * The guide is static text, so the interesting failure mode is not "does it
+ * render" but **drift**: it names tools, charge types and fields, and a rename
+ * elsewhere would silently leave the model reading instructions for a surface
+ * that no longer exists. These tests pin it to the code it describes.
+ */
+
+const PRINCIPAL: AuthPrincipal = {
+  subject: 'user-1',
+  issuer: 'https://tenant.auth0.com/',
+  audience: 'aud',
+  scopes: [],
+  email: null,
+  expiresAt: undefined,
+  claims: { sub: 'user-1' },
+};
+
+function authContext(businessIds: string[]): McpAuthContext {
+  return buildAuthContext(
+    PRINCIPAL,
+    businessIds.map(businessId => ({ businessId, roleId: 'accountant' })),
+  );
+}
+
+/** Fails the test if anything reaches upstream — the handler must be pure. */
+function forbiddenClient() {
+  const fetchImpl = vi.fn(async () => {
+    throw new Error('the data-model guide must not call upstream');
+  });
+  return new UpstreamGraphQLClient({
+    endpoint: 'http://localhost:4000/graphql',
+    timeoutMs: 1000,
+    fetchImpl: fetchImpl as unknown as typeof fetch,
+  });
+}
+
+function run(auth: McpAuthContext, rawArgs: unknown = {}) {
+  return executeRegisteredTool({
+    tool: dataModelGuideTool,
+    rawArgs,
+    auth,
+    correlationId: 'corr-1',
+    client: forbiddenClient(),
+    authorization: 'Bearer tok',
+  });
+}
+
+describe('dataModelGuideTool', () => {
+  it('returns the guide as text and structured content without calling upstream', async () => {
+    const result = await run(authContext(['b1']));
+
+    expect(result.isError).toBeUndefined();
+    expect(result.content[0]!.text).toBe(DATA_MODEL_GUIDE);
+    const structured = result.structuredContent as { guide: string; version: string };
+    expect(structured.guide).toBe(DATA_MODEL_GUIDE);
+    expect(structured.version).toMatch(/^\d+\.\d+\.\d+$/);
+  });
+
+  // This is the point of the tool: it has to be readable at cold start, before
+  // the model knows which business it is working with — or whether it has one.
+  it('works for a caller with no memberships at all', async () => {
+    const result = await run(authContext([]));
+    expect(result.isError).toBeUndefined();
+    expect(result.content[0]!.text).toBe(DATA_MODEL_GUIDE);
+  });
+
+  it('rejects unknown arguments', async () => {
+    const result = await run(authContext(['b1']), { businessId: 'b1' });
+    expect(result.isError).toBe(true);
+    expect((result.structuredContent as { code: string }).code).toBe('VALIDATION_ERROR');
+  });
+
+  it('is registered ahead of every data tool', () => {
+    const order = toolRegistry.describe().map(tool => tool.name);
+    const guideIndex = order.indexOf(DATA_MODEL_GUIDE_TOOL_NAME);
+    expect(guideIndex).toBeGreaterThanOrEqual(0);
+    expect(guideIndex).toBeLessThan(order.indexOf('accounter_list_accounts'));
+    expect(guideIndex).toBeLessThan(order.indexOf('accounter_search_charges'));
+  });
+});
+
+describe('data-model guide content stays in sync', () => {
+  // A tool rename would leave the guide telling the model to call something that
+  // does not exist — the single most likely way this text goes stale.
+  it('only references tools that are actually registered', () => {
+    const registered = new Set(toolRegistry.list().map(tool => tool.name));
+    const referenced = [...DATA_MODEL_GUIDE.matchAll(/accounter_[a-z_]+/g)].map(match => match[0]);
+
+    expect(referenced.length).toBeGreaterThan(0);
+    for (const name of new Set(referenced)) {
+      expect(registered.has(name), `guide references unregistered tool ${name}`).toBe(true);
+    }
+  });
+
+  it('documents every charge type the connector can emit', () => {
+    const chargeTypes = KNOWN_CHARGE_TYPENAMES.map(typename => chargeTypeFromTypename(typename)!);
+    expect(chargeTypes.length).toBeGreaterThan(0);
+    for (const chargeType of chargeTypes) {
+      expect(DATA_MODEL_GUIDE, `guide omits chargeType ${chargeType}`).toContain(chargeType);
+    }
+  });
+
+  it('documents every account type', () => {
+    for (const accountType of [
+      'BANK_ACCOUNT',
+      'BANK_DEPOSIT_ACCOUNT',
+      'CREDIT_CARD',
+      'CRYPTO_WALLET',
+      'FOREIGN_SECURITIES',
+    ]) {
+      expect(DATA_MODEL_GUIDE).toContain(accountType);
+    }
+  });
+
+  // The traps the feedback session actually fell into. If a future edit trims
+  // the guide, these are the sentences that must survive.
+  it('keeps the warnings that cost the most to rediscover', () => {
+    // Card rows are settled again by a bank row.
+    expect(DATA_MODEL_GUIDE).toMatch(/double-count/i);
+    // Internal transfers are neither income nor expense.
+    expect(DATA_MODEL_GUIDE).toContain('internal_transfer');
+    // Securities rows have no mirror leg.
+    expect(DATA_MODEL_GUIDE).toMatch(/single-legged/i);
+    // Per-transaction balances are unavailable, and cumulativeNet is not one.
+    expect(DATA_MODEL_GUIDE).toContain('cumulativeNet');
+    expect(DATA_MODEL_GUIDE).toMatch(/balances are not available/i);
+    // Historical, not present-day, exchange rates.
+    expect(DATA_MODEL_GUIDE).toContain('exchangeRate');
+    // Overlap-vs-containment date semantics.
+    expect(DATA_MODEL_GUIDE).toContain('fromAnyDate');
+  });
+
+  // The whole guide is prompt budget the model pays on every read, and it is
+  // cheap to keep appending to. Currently ~5.8KB; this is the ceiling before an
+  // edit should be trading content out rather than adding.
+  it('stays within its prompt budget', () => {
+    expect(Buffer.byteLength(DATA_MODEL_GUIDE, 'utf8')).toBeLessThan(6_500);
+  });
+});

packages/mcp-server/src/tools/__tests__/scope-forwarding.test.ts

@@ -3,6 +3,7 @@ import { buildAuthContext, type McpAuthContext } from '../../auth/identity.js';
 import type { AuthPrincipal } from '../../auth/token.js';
 import { BUSINESS_SCOPE_HEADER, UpstreamGraphQLClient } from '../../upstream/graphql-client.js';
 import { LIST_BUSINESS_MEMBERSHIPS_TOOL_NAME } from '../businesses.js';
+import { DATA_MODEL_GUIDE_TOOL_NAME } from '../data-model-guide.js';
 import { executeRegisteredTool } from '../execute.js';
 import { toolRegistry } from '../registry-instance.js';
 
@@ -130,6 +131,12 @@ const ARGS_BY_TOOL: Record<string, unknown> = {
   accounter_ledger_records: { businessId: B1 },
 };
 
+/**
+ * Tools with pure handlers: discovery (memberships are already on the auth
+ * context) and the static data-model guide. Neither talks upstream.
+ */
+const PURE_TOOLS = new Set([LIST_BUSINESS_MEMBERSHIPS_TOOL_NAME, DATA_MODEL_GUIDE_TOOL_NAME]);
+
 /** A tool that took a singular `businessId` must have narrowed the scope to it. */
 function expectedScopeFor(name: string): string[] {
   const args = ARGS_BY_TOOL[name] as { businessId?: string } | undefined;
@@ -159,8 +166,10 @@ describe('registry-wide business-scope forwarding', () => {
       expect(result.isError, `${name} should succeed`).toBeUndefined();
       const structured = result.structuredContent as Record<string, unknown>;
 
-      if (name === LIST_BUSINESS_MEMBERSHIPS_TOOL_NAME) {
-        // Discovery is pure and *is* the scope — no upstream call, no echo.
+      if (PURE_TOOLS.has(name)) {
+        // These make no upstream call, so there is nothing to forward and no
+        // scope to echo. Asserted rather than skipped: if one of them ever grows
+        // an upstream call, it must come back through the scope contract above.
         expect(headersSeen).toHaveLength(0);
         expect(structured).not.toHaveProperty('scope');
         return;