Repository navigation
feat: worked TypeScript reference for Context Retriever over a retail-banking model - #1
Merged
Merged
Conversation
This commit adds a complete reference application over a synthetic retail-banking model: schema declaration, provisioning, retrieval, guardrail demonstration, cross-entity composition, and a tool-calling agent behind a single-file web interface. The SDK ships a good README and example scripts. What it does not ship is those pieces assembled against a realistic data model, which is where the mistakes happen. Each non-obvious decision is explained where the code makes it, so the repository reads as guidance rather than as a sample to copy blindly. Three properties are evidenced rather than asserted. Retrieval latency is measured client-side and reported per call. Guardrails rest on the schema, so a constrained field refuses an invented value at the service rather than in a prompt. Token cost is reported per question, and the aggregate tools answer "how much" and "how many" without returning rows. Notable decisions recorded in the code: - Two entities, not five. Relationships generate no traversal tool, so every hop is a caller round trip charged in latency and tokens. An entity earns its place by carrying enough attributes to answer a question of its own. - Twelve of the transaction's fields are indexed, deliberately. Only tag-indexed and numeric-indexed fields become filter parameters, and tool schemas are re-sent every turn, so the index set is the lever on per-turn cost. - Key-component fields carry no index, because the key is the address. - Identifiers are stored as strings. The source type permits values above Number.MAX_SAFE_INTEGER, which JSON.parse would silently corrupt. - The agent is forbidden from totalling money itself. Every total it reports is a figure the engine returned. - Counts over a filtered set read total_count with a limit of one, which is exact and costs a single record. - The customer profile arrives from an argument-free internal tool. A tool accepting a customer identifier would let the agent pivot to another customer. Every organisation, bank and identifier in the seeded data is invented, and the bank codes are BIC-shaped but unregistered.
This commit adds `npm run probe`, which reports what the agent's tool surface cannot express. It calls no model, so its result is a property of the deployment rather than a sample of one model's behaviour on the day. Every other demonstration in the repository shows the system doing something. This one shows what it cannot do, which is the harder claim to make and the one an evaluation cares about. Four classes of read are checked against the discovered tool contracts: a filtered aggregate, a grouping over two dimensions at once, any mutation, and any raw query syntax passed through an argument. The output separates structural boundaries from soft ones. A structural boundary survives a jailbreak, a prompt edit and a model upgrade, because no argument expresses the read. A soft boundary is a sentence in a system prompt that the model can be argued out of. Both look identical in a transcript, so three prompt-held rules are listed alongside the four structural ones, each naming what holds it. User scoping is neither, and it is reported as such. Nothing in the contract stops the caller asking for another customer's rows: the filter tool accepts a customer_id condition and takes any value. The ACL stops the read one layer down, so the probe executes it instead of asserting it, and reports the server's own total_count for two calls differing in one string. The probe exits non-zero if a foreclosed read turns out to be expressible. A correctly scoped surface can never exercise that path, so the detection is covered by unit tests against synthetic mis-scoped surfaces rather than left to be trusted.
`npm run query --tool` looked up the result schema for the domain's subject entity regardless of which tool was named, so calling a beneficiary tool validated beneficiary records against the transaction schema and reported every one of them as malformed. The output read as a service fault when it was the caller looking up the wrong schema. The entity now follows the tool name, using the same convention the rest of the code derives tool names from, and any entity other than the subject gets a generic projection rather than one shaped for transactions. Also records the enum guardrail firing unprompted in the README. The agent filtered on an invented status value on roughly half its attempts, and the service refused it and named the permitted set. An intermittent failure that the contract catches every time is the clearest argument for putting a constraint in the schema rather than in an instruction.
This commit adds `npm run bench`, which runs the seven questions in `src/agent/questions.ts` and reports tokens, model calls, retrieval calls and latency per question and in aggregate. Two of the seven are questions this surface cannot answer: a benchmark of only the flattering cases reports a figure no adopter will reproduce. The agent's ask path moves out of the HTTP server into `src/agent/ask.ts`, so the benchmark and the web interface measure the same code rather than two implementations that agree by inspection. Two findings came out of running it, both measured over 21 turns. Retrieval is about a tenth of one per cent of a turn: a median of 11.5 ms per call against a median whole-turn time of 10.31 s. The model is the entire latency budget, and the README now says so. The useful claim is not that retrieval is fast enough, it is that retrieval never becomes the thing anyone has to optimise. The cold-against-warm separation the requirement asked for turns out not to matter. The first turn is measurably more expensive, 11.21 s against a 10.23 s median, but the premium sits inside the model's own variance. It matters to a deployment that opens a session per request, which is what SurfaceSession exists to prevent. Ranking counterparties exposed an N+1 the aggregate could not avoid on its own: aggregates group by identifier, so the agent resolved a name per group, taking eleven retrieval calls. The instruction now requires it to decide which few matter from the numbers before looking up any name, which took the same question to six calls and halved its retrieval time.
Seven reviews, each from one perspective. The security review found no
must-fix and rejected both of its own candidates on verification. The
rest found two silent-failure bugs, an unguarded route to the data, and
several claims this repository was making that its code did not support.
Two bugs that failed silently, which is why neither had been noticed.
`DomainModel.generate` took only a seed, so the retail-banking domain
seeded its default customer while `provision` scoped the agent key to
`DEMO_CUSTOMER_ID`. Setting that variable to anything other than the
default, which `.env.example` invites, seeded one customer and scoped
the key to another: every command returned nothing, and the README
explains that symptom away as an index that failed to build. The
interface now carries the subject value, and provisioning passes the
same value it scopes the key to, so the two cannot disagree.
Provisioning also hardcoded the access tag as "customer" while the
entity declares it on its own ACL mapping. A domain declaring any other
tag minted a key matching nothing, with the same invisible symptom. The
tag is now derived from the mapping by `subjectAccessTag`, which throws
if the subject field has no mapping at all.
`dataset_overview` is removed rather than fixed. It counted records by
scanning the keyspace over the seeding credentials, so it reported every
customer's totals to an agent scoped to one customer, and it put raw
keyspace access on the request path in a demonstration whose claim is
that the agent reaches data only through the surface. `count_transaction`
answers the same question and is scoped.
`SurfaceSession` had no in-flight guard, so two concurrent requests both
connected and one client was orphaned without being closed. Concurrent
callers now share one connect.
Guardrail 1 announced that enumerated values are a closed set and then
attempted an unknown *field*, which trips a different check. It now
sends a disallowed value to the service and prints the refusal, which is
the mechanism it was always claiming.
`transaction_code` and `channel_code` both carry POS and ATM, and their
descriptions did not distinguish them. Asked how many card payments were
made last month, the agent picked the channel and answered zero where
the answer was eight. Both descriptions now say which questions they
answer, in the field descriptions rather than the system prompt, because
those travel with the schema.
Claims corrected: fourteen fields are indexed and twelve filterable, not
twelve or thirteen; the ACL mapping *is* what enforces scoping, in the
one file a reader will copy; a domain swap does not leave everything
else unchanged, and the README now says which three things it costs; and
the two latency medians in the README are different measurements, now
labelled as such.
Also: the other-customer identifier is derived from the generated data
rather than hardcoded, so setting DEMO_CUSTOMER_ID to it no longer makes
`probe` report a false finding; `.env.example` carries the data-source
variables `provision` requires, without which the documented setup could
not work; the web interface escapes the identity values and defines its
escape helper before first use, where a const in temporal dead zone
would have thrown; the server binds loopback explicitly rather than every
interface while printing "localhost"; `@redis/json` is declared rather
than relied on transitively; `entityOf` prefers the longest match; and
the stray `export { formatMs }` left by the ask extraction is gone.
… .env Acts on the testing review. The suite goes from 32 tests to 59, and the seven it lost were the ones that could not fail for a reason anyone cares about. Deleted: a check that every field type is one the engine accepts, when every field is built through a helper that already takes a FieldType; a roll call of the two internal tool names; a separate assertion that `dr_cr_flag` is unindexed, which the curated-twelve test already fails on. Collapsed: three identifier round-trip cases into one loop, and three one-line fixture facts into a single test that says why the demo depends on each of them. Added, in the order the cost of the bug would be felt. `conditions.ts` is the code an adopter is most likely to copy, and every one of its failure modes is silent or misattributed. Thirteen tests now pin it: the refusal names the permitted set, both condition arrays count against one cap, bounds are renamed to `min_value`/`max_value`, `exclude` is omitted unless true, `limit` is clamped to the page cap, and a sort the tool cannot perform is dropped while one it can but does not accept is refused. `retrieve.ts` is where two live bugs came from. The engine does not populate `structuredContent`, so the payload is parsed out of a text block; and a plain-text error envelope must yield no records *and* no problems, or a refusal reads as corrupt data. The known limit of `isAggregate` — any payload with a field named `value` looks like an aggregate — is recorded as a passing test rather than left to be rediscovered. `config.test.ts` covers the two properties a public repository cannot get wrong: every missing variable is reported in one pass, and no message ever contains a value, asserted against a planted secret. It also covers `entityOf` preferring the longest match, and the two scoping values now derived from the domain rather than hardcoded. The suite also stops reading the developer's `.env`. Importing the config module used to load it as a side effect, so tests inherited real credentials merely by importing something that transitively reached there — and the first configuration test proved it by passing when it should have failed. The load is now lazy, the Vitest config sets `DEMO_SKIP_DOTENV`, and the pure foreclosure logic moved to `src/data-plane/foreclosure.ts` so the probe tests pull in no configuration at all. Verified by moving `.env` aside: 59 tests green. Two claims removed rather than kept. The README and the CI workflow both said tests needing a live deployment skip themselves; no such test or skip helper exists, and the honest statement is that no test reaches a service, a credential or the network. The README's development command also omitted `format:check`, which CI runs, so a first contribution could pass locally and fail there.
The README explains the code and serves a handover. Neither of those is what somebody needs while standing in front of an audience, and the running order that makes this demo land existed only in one person's head. DEMO.md is that script: a pre-flight checklist where every item has failed at least once, five steps with the point to make at each, the real latency answer, what to do when a turn varies, and a closing list of things not to promise. Two parts carry most of the value. The pre-flight says to ask one throwaway question off-screen, because the first turn of a process pays for the MCP connection and the first tool discovery. And step four stages the contrast pair in the order that works: "how many card payments last month" is exact and costs one record, and only then "how much", which is correctly declined. Asked the other way round the refusal reads as a missing feature rather than as an agent that knows its own contract. The web interface now lists those two questions adjacently and in that order, with the reason recorded next to the list. Every figure in the script was re-run rather than remembered: fourteen tools, 322 against 0 on the scoped read, nineteen failed rows, six retrieval calls for the counterparty ranking, and the guardrail summary naming two of five as the caller's job.
The adopter is comparing this against running text-to-SQL on Oracle, where arithmetic is the database's job and is exact. That voids an argument the demo script was about to make and changes what the aggregate limitation is. The script previously answered the refusal in step 4 by observing that an agent totalling rows itself gets the sum wrong, measurably and repeatedly. Against a comparator that computes SUM in the database, that argues with a position nobody holds, and presenting it would read as misdirection. Removed. The measurement still justifies the prompt rule forbidding the agent from totalling rows, and nothing beyond that. More importantly, a conditionless aggregate is not a neutral boundary to explain honestly. It is the one axis where SQL wins outright, and it is the thing the adopter will remember. The script now says to concede filtered aggregation and multi-field grouping as gaps rather than design choices, and to concede analytical work generally, before moving on. A new section gives the comparison that is actually defensible, which trades expressiveness for enumerability. Tenant isolation is declarative here and procedural there, so the question to ask is how their SQL agent authenticates to Oracle: a service account is the whole argument. Every read this surface can perform is enumerable in advance, which is what `npm run probe` prints and what a model-risk function has to sign off. Blast radius is bounded by the contract rather than by intent, since a read-only role does not protect availability against a full scan. And the lever is model round trips, where a SQL agent spends turns inspecting schema and recovering from a wrong column. One claim in that section is marked as needing verification before it is said aloud, rather than asserted: whether the SQL toolkit they use puts sample rows in its schema output, which would be both a token cost and an out-of-scope read that auditing generated queries would miss.
The README never said what Context Retriever is. It opened by naming the SDK and then, in its third paragraph, described a compound query across two entities in a single sentence that nobody could follow without already knowing the model. The front of the document is now three sections in the order a new reader needs them. What Context Retriever is: you declare a data model, the service builds the indexes and generates typed MCP tools, and the agent calls those tools rather than writing queries. Two consequences follow, which are the reason to use it at all, and a table defines the six terms the rest of the document assumes. What this repository is: the SDK's pieces assembled against a realistic model, and the three reasons somebody might read it. What is inside: every command with one line on what it does, then the source tree. The confusing sentence is replaced by a plain statement of what the worked example is, and the cross-entity composition it referred to is described where that command is documented. Structure follows the conventions in the Iris demos repository: a centred title block with a one-line tagline, tables rather than prose for anything the reader scans, links to the product documentation, and the licence last. The duplication this introduced has been removed rather than left. The Setup section no longer re-lists commands the contents table already covers, and the Layout section is now "Two planes, two credentials", keeping the reasoning about credential separation and dropping the tree. Validated against the house style: one em dash in the document, no phrase-level tells, no negative parallelism, no American spellings in prose, and every section opening with its conclusion. The three remaining checker findings are false positives, being an HTML align attribute and two proper nouns. The two redis.io links return 200; the npm and MCP links could not be checked from here.
Matches the header convention used across the Redis repositories: a centred block with the logo, an `<h1>` title and a bold one-line tagline, followed by the descriptive sentence and a rule. The image is the same `raw.githubusercontent.com` SVG that redis-vl-python and adk-redis reference, at the 300px width redis-vl-python uses, rather than a copy committed here. That keeps one canonical asset, at the cost of a cross-repository dependency: if that path moves, this header breaks along with several other repositories. The title moves from a Markdown heading to an `<h1>`, which is what the convention uses and what keeps it inside the centred block.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07251d7bd5
ℹ️ 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".
Six review comments, all six legitimate. Two of them turned out to describe the smaller half of a real problem. ### Compose was mislabelling its own output `compose` asked how much had been sent to matching counterparties and answered with an unfiltered aggregate, which covers credits as well as debits and failed attempts alongside settled payments. The aggregate tools take no conditions, so narrowing it is not available; the label was the thing that could be fixed, and it now says precisely what the figure is and what it is not. The command then shows what *is* exactly answerable about a subset: two filtered counts, read from `total_count` with `limit: 1`. A filtered count is free and a filtered sum is unavailable, which describes the boundary more sharply than calling aggregates limited. ### A counterparty now settles in one currency The generator drew a currency per transaction, so a single foreign counterparty held both USD and EUR rows. That made a sum grouped by `beneficiary_id` a cross-currency total and falsified what `compose` said about its own output. Currency is a property of the counterparty, so the fixture models it that way. Verified: no beneficiary carries more than one currency, and the set still spans three. ### The retrieval median was computed over the wrong population `bench` averaged each turn's retrieval calls and took the median of those averages, giving a turn with one call the same weight as a turn with eleven, and reporting the turn count as a call count. It now flattens to individual durations: the median moved only from 11.5 ms to 11.4 ms, but the sample is 40 calls rather than 18 turns. The share of a turn is likewise computed per turn and then reduced, rather than dividing one median by another. Re-measuring falsified a claim in the README. The cold-against-warm comparison inverted, with the first turn at 9.62 s against an 11.84 s median for the rest, so the README no longer asserts that the first turn is measurably more expensive. One cold sample cannot outweigh the model's own variance, and the useful point survives: a connection paid once per process disappears into the noise, and would not for a deployment opening a session per request. ### Surface names are not unique, which is worse than stale reuse The review noted that provisioning reused a same-name surface without comparing its model. Investigating it found the assumption underneath was wrong: the service does not enforce unique names, so creating a surface whose name is taken succeeds. Provisioning did not silently reuse a stale surface, it silently created a second one, and any later lookup by name then picked one of several arbitrarily. Three duplicates were created while confirming this. Provisioning now looks up before creating, reuses only after comparing a structural fingerprint of the model, and refuses outright when more than one surface already carries the name. The fingerprint covers entity names, field names and ACL mappings, because those decide the tool surface and who can read what; the whole model is not compared, since the service may normalise what it stores. Seven tests cover the check, including the dangerous case where the fields match and the access tag does not. Teardown shares the paginated lookup rather than reading page one, which previously reported "nothing to do" for a surface on a later page, and reports and deletes every surface matching the name rather than an arbitrary one. ### Repeat now works for aggregate tools `npm run query --tool <aggregate> --repeat N` returned after reporting the first result and never printed the promised distribution.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The Context Retriever TypeScript SDK ships a README and five example scripts. What it does not ship is those pieces assembled into a working application against a realistic data model, which is where the mistakes happen: conflating the two credential types, believing a half-failed import succeeded, minting a once-only agent key and then throwing before storing it, or curating an index set without knowing that every indexed field's description becomes agent-facing prompt text charged on every turn.
This repository is that assembly, with each non-obvious decision explained where the code makes it. It exists to be read and copied, so it is deliberately small enough to review in an afternoon and complete enough to run:
npm run typecheck && npm run lint && npm run format:check && npm testall pass on a clean checkout with nothing configured, and no test reaches a service, a credential or the network.Three claims are evidenced by a command rather than asserted in prose: retrieval latency, guardrails, and token cost. Every figure quoted in the documentation was printed by the command named beside it.
Changes
A worked retail-banking data model
Two entities,
transactionandbeneficiary, over synthetic records. Three entities a first pass would have modelled (customer, account and channel) are collapsed ontotransactionas fields, because relationships generate no traversal tool and so every hop is a caller round trip charged in latency and tokens.Fourteen of the transaction's twenty-five fields carry an index and twelve of those are tag- or numeric-indexed, which is the number that matters: only tag and numeric indexes become filter parameters. Three type mappings are decisions rather than translations, and each is explained at the field. Twenty-digit identifiers are held as strings, because the source type permits values above
Number.MAX_SAFE_INTEGERthatJSON.parsewould silently corrupt.Every organisation, bank and identifier in the seeded data is invented, and the BIC-shaped codes are synthetic rather than registered.
Control plane and data plane, separated structurally
src/control-plane/andsrc/data-plane/are separate directories with no import between them. They speak different protocols and take different credentials, and the SDK documents mixing them up as the usual cause of a puzzling 401. Keeping them apart makes holding the wrong credential structurally impossible rather than something to remember.The provisioning path waits for
activeexactly, treating an unrecognised status as still in flight so a new status costs a timeout instead of a wrong answer; inspects the import response, becauseimportDatareports per-record failure rather than throwing; and captures the agent key before anything else can fail, because the service returns it once.A deterministic capability probe
npm run probereports what the tool surface cannot express, calling no model, so its result is a property of the deployment rather than a sample of one model's behaviour on the day. It separates four structurally foreclosed reads from three rules that live in a sentence of the system prompt, because both look identical in a transcript and only one survives a jailbreak or a model upgrade.User scoping is neither, and is reported as such. Nothing in the tool contract stops a caller asking for another subject's rows, so the probe issues that request and reports the server's own match count for both: the request is well formed, accepted, and answered with nothing. The probe exits non-zero if a foreclosed read turns out to be expressible, and because a correctly scoped surface can never exercise that path, the detection is covered by unit tests against synthetic mis-scoped surfaces.
An agent, and a benchmark that measures the same code
src/agent/bridge.tsmerges the retrieval tools discovered from the surface with argument-free internal tools the application supplies. Nothing hard-codes a generated tool name. A tool result is treated as data: results reach the model only as tool output, nothing retrieved is interpolated into a system prompt, and the server-supplied descriptions are used for discovery, never as trusted configuration.npm run benchruns a committed question set and reports tokens, model calls, retrieval calls and latency per question. Two of the seven questions are ones this surface cannot answer, because a benchmark of only the flattering cases reports a figure nobody will reproduce. The ask path lives insrc/agent/ask.tsso the benchmark and the web interface measure the same code rather than two implementations that agree by inspection.Documentation
README.mdexplains what Context Retriever is before what this project is, defines the vocabulary the rest of the document assumes, and lists every command and what it does.DEMO.mdis a script for showing the repository live: a pre-flight checklist, five steps with the point to make at each, the honest latency answer, and a closing list of things not to promise.Notes
The measured headline is not the one this repository was built expecting. Retrieval takes a median of 11.5 ms per call while a whole agent turn takes a median of 10.31 seconds, so retrieval is around a tenth of one per cent of the time a question costs. The documentation states that plainly rather than burying it, and draws the conclusion that follows: the model is the latency budget, so the lever that moves is the number of model round trips an answer takes, and the tool surface is what decides that.
Two limits shape what the worked example can demonstrate, and both are documented rather than worked around. The aggregate tools accept no conditions and group by a single field, so there is no filtered sum, and the agent correctly declines a question asking for one. Counting a filtered set is the exception and is exact, because filter and search results carry a server-side match count independent of the page size.
The demo authenticates nobody: the signed-in subject comes from configuration, and in production a validated token supplies it. Scoping is still enforced, by the ACL mapping against the access tags on the agent key, so this shows one tenant properly isolated rather than a multi-tenant identity flow.
Next Steps
A conformance kit, so an adopter can run this repository's domain-model assertions against their own
DomainModelrather than reading the retail-banking ones and adapting by eye. Most of the assertions already exist and are hardcoded to one domain.Schema inference as a reviewable command is deliberately deferred rather than skipped for want of time.