Implement Sheets-backed client memory layer - #30
Conversation
Specifies a Sheets-backed client-memory layer for DIA, built on the sheets-mini-db pattern proven this session (INSERT_ROWS-safe multi-writer Sheet, LockService for read-modify-write). Data model, read/write paths, architecture touchpoints, and 6 open questions -- deliberately spec-only, implementation deferred.
Implements docs/2026-07-07_client-memory-layer_design.md: DIA now recalls repeat clients and records every scored intake in an operator-owned Google Sheet (clients rollup + engagements append-only ledger). - client_discovery/memory.py: ADC auth, INSERT_ROWS-only writes (the only primitive proven safe under concurrency), ledger-before-rollup ordering, full degrade-to-warning posture — MEMORY_SHEET_ID unset means zero change. - character.py: recall_client_history plain tool (auto-included when memory is configured) + engagement recording on the agent's document path. - app.py: /api/process records after the response is on the wire, so a slow Sheet can never delay an intake. - 16 new tests; full suite 100 passed / 2 skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec5909fe92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| client = next( | ||
| ( | ||
| c | ||
| for c in clients | ||
| if c.get("client_id", "").split("|")[0] == name_only |
There was a problem hiding this comment.
Only use name-only recall when the website is absent
When an intake supplies a website but there is no exact name|domain match, this fallback still returns the first row with the same normalized name. In a sheet containing acme|old.example, a new intake for Acme at new.example will be treated as the old client and the agent can reference another company's prior pain points/status; the name-only fallback should be limited to calls where the incoming website is blank.
Useful? React with 👍 / 👎.
| from client_discovery import memory | ||
| from client_discovery.models import ClientIntake, OpportunityScore | ||
|
|
||
| if not memory.is_enabled(): |
There was a problem hiding this comment.
Load .env before gating /api/process memory writes
For the documented local flow (python app.py with MEMORY_SHEET_ID in .env), posting directly to /api/process reaches this check before any module has loaded dotenv; app.py imports client_discovery.config but never calls its loader, and only the agent path loads .env. As a result the deterministic path silently skips recording every intake until /api/agent or /api/agent/status happens to import agent_runtime, so MEMORY_SHEET_ID should be loaded before this is_enabled() gate (or inside the memory config helper).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds an optional, Google Sheets–backed “client memory” layer to persist and recall prior client engagements across intakes, gated by MEMORY_SHEET_ID so default behavior remains unchanged when unset.
Changes:
- Introduces
client_discovery/memory.pyto read/write aclientsrollup tab and an append-onlyengagementsledger tab via the Sheets API (ADC auth). - Wires memory recording into
/api/process(after responding) and the agent document-generation path, and exposes arecall_client_historytool when enabled. - Documents setup and adds tests covering identity, read/write behavior, failure isolation, and tool gating.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
client_discovery/memory.py |
Implements Sheets-backed read/write memory layer and client identity normalization. |
character.py |
Records agent-path engagements and conditionally adds the recall tool + instruction update. |
app.py |
Records deterministic /api/process engagements after sending the HTTP response. |
tests/test_memory.py |
Adds coverage for identity, write ordering, failure handling, reads, and agent tool gating. |
README.md |
Adds setup/behavior documentation for the optional Sheets-backed memory. |
docs/2026-07-07_client-memory-layer_design.md |
Adds the design/spec document referenced by the implementation. |
.env.example |
Documents MEMORY_SHEET_ID configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Writes use ``insertDataOption=INSERT_ROWS`` only. Naive append (OVERWRITE) | ||
| silently loses concurrent writes (9/12 lost in the 12-writer test); | ||
| INSERT_ROWS survived 12/12. |
| import uuid | ||
| from dataclasses import asdict | ||
| from datetime import datetime, timezone |
| def _rows_as_dicts(rows: list[list[str]], headers: list[str]) -> list[dict[str, str]]: | ||
| """Map sheet rows to dicts by the tab's actual header row. | ||
|
|
||
| The first row of the tab is trusted as the header so operator-added | ||
| columns (or reordered ones) don't corrupt field mapping. | ||
| """ | ||
| if not rows: | ||
| return [] | ||
| actual_headers = [h.strip() for h in rows[0]] or headers | ||
| out = [] | ||
| for row in rows[1:]: | ||
| padded = row + [""] * (len(actual_headers) - len(row)) | ||
| out.append(dict(zip(actual_headers, padded))) | ||
| return out |
Summary
Implements docs/2026-07-07_client-memory-layer_design.md — XPRIZE roadmap item 1. DIA now recalls repeat clients and records every scored intake in an operator-owned Google Sheet.
client_discovery/memory.py— Sheets store: ADC auth,INSERT_ROWS-only writes (the only primitive proven safe under concurrency in the sheets-mini-db proof), engagements-ledger-before-clients-rollup ordering, degrade-to-warning everywhere.character.py—recall_client_historyplain tool, auto-included whenMEMORY_SHEET_IDis set; agent path records engagements on document generation; instruction updated.app.py—/api/processrecords after the response is on the wire, so a slow Sheet can never delay an intake..env.example.Spec decisions resolved
Identity = normalized name + domain composite key · failed intakes not recorded · v1 accepts write loss on Sheet outage (logged) · single-tenant · plain tool over MCP (Open Q6, MCP flagged as future branch).
Safety posture
MEMORY_SHEET_IDunset → byte-identical behavior to today, zero new dependencies (google-auth already ships with ADK). Client data goes only to the operator's own Sheet — never shared infrastructure.Test plan
tests/test_memory.py(identity, INSERT_ROWS enforcement, write ordering, failure isolation, read matching + fallback, agent tool gating)🤖 Generated with Claude Code