From 6ba7045dccc96213fe749905c741ea01f826cae4 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 20 Aug 2026 18:34:56 +0100 Subject: [PATCH 1/5] adaptor docs knowledge tests --- .../bugs/test_repro_gmail_sendmessage_keys.md | 99 ++++ .../integration/adaptor_knowledge/README.md | 162 +++++++ .../integration/adaptor_knowledge/__init__.py | 0 .../integration/adaptor_knowledge/cases.py | 440 ++++++++++++++++++ .../integration/adaptor_knowledge/conftest.py | 90 ++++ .../adaptor_knowledge/scoreboard.py | 39 ++ .../adaptor_knowledge/seed_docs.py | 240 ++++++++++ .../test_adaptor_knowledge.py | 103 ++++ 8 files changed, 1173 insertions(+) create mode 100644 services/job_chat/tests/acceptance/bugs/test_repro_gmail_sendmessage_keys.md create mode 100644 services/job_chat/tests/integration/adaptor_knowledge/README.md create mode 100644 services/job_chat/tests/integration/adaptor_knowledge/__init__.py create mode 100644 services/job_chat/tests/integration/adaptor_knowledge/cases.py create mode 100644 services/job_chat/tests/integration/adaptor_knowledge/conftest.py create mode 100644 services/job_chat/tests/integration/adaptor_knowledge/scoreboard.py create mode 100644 services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py create mode 100644 services/job_chat/tests/integration/adaptor_knowledge/test_adaptor_knowledge.py diff --git a/services/job_chat/tests/acceptance/bugs/test_repro_gmail_sendmessage_keys.md b/services/job_chat/tests/acceptance/bugs/test_repro_gmail_sendmessage_keys.md new file mode 100644 index 00000000..e7f6fc39 --- /dev/null +++ b/services/job_chat/tests/acceptance/bugs/test_repro_gmail_sendmessage_keys.md @@ -0,0 +1,99 @@ +--- +id: job-chat.tmp.repro-gmail-sendmessage-keys +service: job_chat +runs: 5 +judges: [general, openfn_code_quality] +--- + +# notes + +Reproduction of a user report: when asked to send mail with the gmail adaptor, +job_chat writes `sendMessage({ to, subject, text: ... })` or +`sendMessage({ to, subject, message: ... })`. Neither key exists on +`SendMessageOptions` — the body key is `body` — so the operation sends an empty +message or throws at run time. The reporter saw it "at least half a dozen +times", hence `runs: 5`: this measures a rate, not a single verdict. + +Why it happens (not a judgement call — verified in the code): + +- `job_chat/prompt.py:generate_system_message` injects ONLY the `signature` + column from `adaptor_function_docs`. For gmail@3.2.0 the entire adaptor + context the model receives is a bare name list, ending in + `sendMessage(message)`. The stored `function_data` JSONB does hold the + param descriptions and the docsite example (which uses `body`), but nothing + reads them. +- `load_adaptor_docs.filter_function_docs` keeps only doclets of kind + `function` / `external-function` / `external`, so the `SendMessageOptions` + typedef — the only place the `body`/`to`/`subject`/`attachments` property + names are defined — is never stored at all. +- `job_chat/retrieve_docs.py:search_docs` pins the docsite RAG to + `docs_type="general_docs"`, so the adaptor docs page can't fill the gap + either. The prompt even says so: "not adaptor-specific APIs, which are + included separately." + +So `body` appears nowhere in the prompt, and the model falls back on its +nodemailer / SendGrid priors, where the body key IS `text` (or `html`, or +`message`). The user's complaint that it ignores "the adaptor doc" is +accurate about docs.openfn.org, but that document never reaches the model. + +Note for local runs: gmail docs must be present in `adaptor_function_docs`, or +the adaptor block degrades to "The user is using an OpenFn Adaptor to write the +job." and the test is no longer a faithful repro. Confirm with +`select signature from adaptor_function_docs where adaptor_name = '@openfn/language-gmail'`. + +Expected behaviour once fixed: the message object uses `body`, and no invented +key. A model that cannot know the key names should say so or ask, not guess +silently. + +# quality_criteria + +- Any `sendMessage` call passes the body text under the key `body`. +- The message object uses no invented key for the body — specifically NOT `text`, `message`, `html`, `content`, or `bodyText`. +- Recipient and subject use the documented keys `to` and `subject`. +- The code calls only functions that exist in the gmail adaptor (`sendMessage`, `getContentsFromMessages`, `getMessageById`) or in language-common; it does not invent a mail-sending function such as `send`, `sendEmail`, or `sendMail`. +- The response does not claim the adaptor supports message fields it has not been shown; if it is unsure of the option names it says so or asks, rather than presenting a guessed key as documented. + +# settings + +## context.expression + +```js +fn(state => { + const failed = state.data.filter(r => r.status === 'error'); + return { ...state, failed }; +}); +``` + +## context.adaptor + +@openfn/language-gmail@3.2.0 + +## context.input + +```json +{ + "data": [ + { "id": "r-1001", "patient": "P-88", "status": "ok" }, + { "id": "r-1002", "patient": "P-91", "status": "error", "reason": "missing dob" }, + { "id": "r-1003", "patient": "P-92", "status": "error", "reason": "bad org unit" } + ] +} +``` + +## suggest_code + +true + +## meta.session_id + +sess-tmp-repro-gmail-sendmessage-keys-0001 + +# turn + +## role + +user + +## content + +now email the failed records to data-team@example.org as a summary, subject "Nightly sync failures" diff --git a/services/job_chat/tests/integration/adaptor_knowledge/README.md b/services/job_chat/tests/integration/adaptor_knowledge/README.md new file mode 100644 index 00000000..9b55bced --- /dev/null +++ b/services/job_chat/tests/integration/adaptor_knowledge/README.md @@ -0,0 +1,162 @@ +# Adaptor knowledge probes + +Thirty cases that ask job_chat something whose correct answer lives in one +specific, named place in an adaptor's documentation, then check the answer with +a regex. They exist to give anyone working on adaptor-docs retrieval a +scoreboard: a number that should go up as the method improves. + +Many of them fail today. That's the point — a suite that already passes +measures nothing. + +## Baseline + +Measured on 2026-08-20, `job_chat` unchanged, docs seeded as described below: + +| Group | Pass | +|---|---| +| `signatures` (controls) | 3/3 | +| `functions` | 4/6 | +| `interfaces` | 4/7 | +| `namespaces` | **1/6** | +| `other` | 2/3 | +| `version` | 2/2 run (3 skipped, old versions unavailable locally) | +| **Total** | **16/27** | + +Treat this as approximate. The model is stochastic and cases near the boundary +flip between runs — `iface.salesforce-bulk-failonerror` passed one run and +failed the next. Re-baseline before and after any change rather than comparing +against these numbers directly. + +`namespaces` at 1/6 is the standout, and the cause is concrete: see the last +section. + +## Running them + +```bash +poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s +``` + +One group at a time: + +```bash +poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s -k interfaces +``` + +Each case costs one job_chat call against the live Anthropic API. The run +prints a per-group scoreboard at the end. + +## Why here, and not under `acceptance/` + +Three reasons, in order of weight: + +**These are pass/fail, not judged.** Every assertion is a regex over the +response. Nobody has to read 30 verdicts to learn what happened — you read one +scoreboard line per group. The `acceptance/` tier is built around +`spec_collector` turning markdown specs into LLM-judged items, which is the +right tool when "is this answer good?" needs judgement, and the wrong one when +the question is "does the string `body:` appear in the generated code?". + +**The tier marker follows the directory.** The repo-root `conftest.py` applies +`unit` / `service` / `integration` / `acceptance` based on which of those names +appears in the test's path. These tests hit a live LLM and Postgres, which is +the repo's own definition of `integration` ("hits real external services... +Manual/nightly"). Putting them under `integration/` gets the correct marker +with no new machinery. + +**Excluding them is automatic.** They aren't in an `acceptance/` directory, so +`spec_collector` never collects them and an acceptance run never touches them. +Nothing to remember, no flag to pass. + +On the nesting question: a topic folder *inside* a tier folder is fine and is +what the marker logic expects. The thing to avoid is the inverse — tier folders +nested under a topic folder — which would still technically work (the root +conftest matches any path segment) but reads backwards. + +If a probe ever needs real judgement rather than a string match, it belongs in +`../../acceptance/` as a markdown spec, alongside +`bugs/test_repro_gmail_sendmessage_keys.md`. Splitting by *how you assert* +rather than by *what you're testing* is what keeps both harnesses simple. + +## Layout + +| File | What it is | +|---|---| +| `cases.py` | The 30 cases as data. Edit this to add or tune probes. | +| `test_adaptor_knowledge.py` | Parametrized runner, regex assertions, scoreboard. | +| `conftest.py` | Skips a case when its adaptor version has no docs. | +| `seed_docs.py` | Dev workaround for machines where jsdoc can't run. | + +## The groups + +| Group | n | Doc location being probed | +|---|---|---| +| `signatures` | 3 | The function list job_chat already injects. **Controls — these should pass.** | +| `functions` | 6 | `## Functions` — parameter names, order, examples | +| `interfaces` | 7 | `## Interfaces` — `@typedef` property names | +| `namespaces` | 6 | `## ` — `tracker.*`, `bulk1/2.*`, `util.*`, `http.*` | +| `other` | 3 | `configuration-schema` and the README | +| `version` | 5 | Behaviour that differs between two pinned versions | + +The `signatures` group is the baseline. If those fail, something is wrong with +the fixture rather than with retrieval. + +Four `version` cases are deliberate inverses of a case in another group — the +same question, a different pin, and the opposite correct answer: + +| Latest-version case | Old-version inverse | +|---|---| +| `fn.http-post-data-positional` (7.3.2, `post(path, data)`) | `ver.http6-post-body-option` (6.5.4, `{body: ...}`) | +| `ns.dhis2-util-findattributevalue` (8.2.1, `util.` prefix) | `ver.dhis2-6-findattributevalue-toplevel` (6.3.4, no prefix) | +| `ns.salesforce-bulk2-insert` (9.1.1, `bulk2.insert`) | `ver.salesforce4-bulk-toplevel` (4.8.6, `bulk()`) | + +A method that just dumps the latest docs will pass one side of each pair and +fail the other. That's the pair's job. + +## Adding a case + +Append a `Case` to the right list in `cases.py`. Fill in `doc_ref` with the +exact section the answer comes from, and `why` with the wrong answer you +expect. Both print on failure, which is what makes a red run actionable rather +than just red. + +Keep `target="code"` for anything that asks for code. Prose discussing a wrong +key ("you might reach for `text:`, but...") would otherwise trip a `forbid`. + +## Adaptor docs have to be present + +A case is only meaningful if job_chat receives a real adaptor block. When the +docs are missing the prompt quietly degrades to "The user is using an OpenFn +Adaptor to write the job.", so `conftest.py` skips those cases rather than +letting them fail for the wrong reason. + +Where `adaptor_apis` works, job_chat auto-loads on first use and there is +nothing to do. Where it doesn't — notably macOS under bun, where jsdoc dies on +`Module.wrapper` (see `JSDOC_BUN_ERROR.md`) — seed the table first: + +```bash +poetry run python services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py +``` + +That pulls the pre-built doclet feed the docsite indexer already uses and +pushes it through the real ingest functions, so the rows match what the live +pipeline would write. It only covers each adaptor's **latest** version, so the +old pins in the `version` group still skip unless that version is already in +your database. + +## What the failures are telling you + +Two upstream causes account for most of them, both verified in the code: + +1. `job_chat/prompt.py` injects only the `signature` column. The + `function_data` JSONB alongside it already holds descriptions, parameter + docs and examples — nothing reads them. +2. `load_adaptor_docs.filter_function_docs` keeps only `function` / + `external-function` / `external` doclets, so `@typedef` blocks — the only + definition of option-object property names — are never stored. + +There is also a third, narrower one worth fixing on its own: the namespace +prefix is stored in `function_name` (`bulk1.insert`) but not in `signature` +(`insert(sObject, records, options)`), and only the signature is injected. The +salesforce block therefore lists `insert(...)` three times with no way to tell +the variants apart, and shows `get`/`post`/`request` as if they were top-level +when they are `http.*`. diff --git a/services/job_chat/tests/integration/adaptor_knowledge/__init__.py b/services/job_chat/tests/integration/adaptor_knowledge/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/services/job_chat/tests/integration/adaptor_knowledge/cases.py b/services/job_chat/tests/integration/adaptor_knowledge/cases.py new file mode 100644 index 00000000..bbf43f20 --- /dev/null +++ b/services/job_chat/tests/integration/adaptor_knowledge/cases.py @@ -0,0 +1,440 @@ +"""Adaptor-knowledge probes: 30 cases, one doc fact each. + +Each case asks job_chat something whose correct answer lives in a specific, +named place in an adaptor's documentation, then asserts on the answer with +plain regexes. No LLM judge — a case passes or fails on a string match, so a +run is a scoreboard you read in one glance rather than 30 verdicts you read +in full. + +`doc_ref` names the exact source of truth. When a case fails, that is where +the missing information lives; it is the spec for whatever retrieval method +you are building. + +Fields +------ +group Which doc location the case probes. Also the pytest sub-id. +adaptor Full specifier, pinned to a version on purpose. Version-sensitive + cases in the `version` group depend on the exact pin. +prompt The user's message. +expression Optional starting job code, as if already in the editor. +target Where the assertions look: + "code" — the suggested code, plus any fenced blocks in the reply. + "text" — the whole reply. + Use "code" whenever the case asks for code; prose about a key name + ("you could pass text:...") would otherwise trip the `forbid` list. +expect Regexes, at least one must match. Empty means "nothing required". +forbid Regexes, none may match. +doc_ref Where the answer is documented. Free text, shown on failure. +why What the model is expected to get wrong, and why. + +All cases run with suggest_code=True. That is what Lightning sends, and it +keeps every case on one prompt path (suggest_code=False builds a different +prompt via old_prompt.py, which would confound the results). +""" + +from dataclasses import dataclass, field + + +@dataclass(frozen=True) +class Case: + id: str + group: str + adaptor: str + prompt: str + doc_ref: str + why: str + expression: str | None = None + target: str = "code" + expect: list[str] = field(default_factory=list) + forbid: list[str] = field(default_factory=list) + empty_target_passes: bool = False + + +# -------------------------------------------------------------------------- +# A. signatures — the function list job_chat already injects today. +# These are controls. They SHOULD pass. If one fails, the baseline is worse +# than we think and that is worth knowing before measuring any improvement. +# -------------------------------------------------------------------------- + +SIGNATURES = [ + Case( + id="sig.gmail-send-function", + group="signatures", + adaptor="@openfn/language-gmail@3.2.0", + prompt="Which function do I use to send an email with this adaptor? Just name it.", + target="text", + expect=[r"sendMessage"], + forbid=[r"\bsendEmail\b", r"\bsendMail\b"], + doc_ref="Functions > sendMessage (in the injected signature list)", + why="Control: the signature list already contains sendMessage(message).", + ), + Case( + id="sig.dhis2-destroy-function", + group="signatures", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="What function deletes a resource in DHIS2? Just name it.", + target="text", + expect=[r"\bdestroy\b"], + forbid=[r"\bdelete\(", r"\bremove\("], + doc_ref="Functions > destroy (in the injected signature list)", + why="Control: destroy() is in the signature list; 'delete' is the natural guess.", + ), + Case( + id="sig.http-request-function", + group="signatures", + adaptor="@openfn/language-http@7.3.2", + prompt="I need to send a HEAD request. Which function supports an arbitrary HTTP method?", + target="text", + expect=[r"\brequest\b"], + forbid=[r"\bhead\("], + doc_ref="Functions > request (in the injected signature list)", + why="Control: request(method, path, options) is in the signature list.", + ), +] + + +# -------------------------------------------------------------------------- +# B. Functions section — parameter names, order and examples. The signature +# gives bare parameter names; everything that disambiguates them is dropped. +# -------------------------------------------------------------------------- + +FUNCTIONS = [ + Case( + id="fn.http-post-data-positional", + group="functions", + adaptor="@openfn/language-http@7.3.2", + prompt="POST the records in state.data to /patients as JSON.", + expect=[r"post\("], + forbid=[r"body\s*:"], + doc_ref="Functions > post — post(path, data, options); data is positional in 7.x", + why="In 6.x the payload went in an options object as `body`. The v7 " + "signature post(path, data, options) does not reveal that `body:` is now wrong.", + ), + Case( + id="fn.dhis2-get-no-callback", + group="functions", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="Fetch all trackedEntities for program IpHINAT79UW and log how many came back.", + expect=[r"get\("], + forbid=[r"callback"], + doc_ref="Functions > get — get(path, params); callbacks removed in 7.0.0", + why="get(path, params) does not say callbacks were removed. Older DHIS2 " + "job code passed a callback as the last argument.", + ), + Case( + id="fn.salesforce-create-takes-array", + group="functions", + adaptor="@openfn/language-salesforce@9.1.5", + prompt="Create three Contact records from state.contacts in a single call.", + expect=[r"create\("], + forbid=[r"each\s*\("], + doc_ref="Functions > create — create(sObjectName, records); records is an Array", + why="The signature says `records` but not that it accepts an array, so the " + "model reaches for each() to loop instead of one bulk-ish call.", + ), + Case( + id="fn.gmail-getcontents-query-key", + group="functions", + adaptor="@openfn/language-gmail@3.2.0", + prompt="Fetch the messages whose subject is 'weekly report'.", + expect=[r"query\s*:"], + # NOT `subject:` — that's legitimate Gmail search syntax *inside* the + # query string, exactly as the docs example writes it. + forbid=[r"\bsearch\s*:", r"\bq\s*:", r"\bfilter\s*:"], + doc_ref="Functions > getContentsFromMessages, Example: query: 'subject:my+test+message'", + why="getContentsFromMessages(options) hides that the search string goes " + "under `query` in Gmail search syntax.", + ), + Case( + id="fn.gmail-getcontents-contents-key", + group="functions", + adaptor="@openfn/language-gmail@3.2.0", + prompt="Download the .xlsx attachment from messages received after 2026/07/01.", + expect=[r"contents\s*:"], + forbid=[r"attachments\s*:"], + doc_ref="Functions > getContentsFromMessages, Example with contents: [{type:'file', file:/\\.xlsx$/}]", + why="`attachments` is the correct key on sendMessage but wrong here — a " + "plausible cross-contamination when neither option object is documented.", + ), + Case( + id="fn.dhis2-create-resource-first-arg", + group="functions", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="Create a new dataValueSet from state.payload.", + expect=[r"create\(\s*['\"]"], + forbid=[r"create\(\s*\{"], + doc_ref="Functions > create — create(path, data, params); path is a resource-type string", + why="`create(path, data)` reads ambiguously; the model may pass a single " + "config object instead of a resource string first.", + ), +] + + +# -------------------------------------------------------------------------- +# C. Interfaces — @typedef property names. Dropped entirely at ingest by +# load_adaptor_docs.filter_function_docs, so none of this reaches the prompt. +# -------------------------------------------------------------------------- + +INTERFACES = [ + Case( + id="iface.gmail-sendmessage-body", + group="interfaces", + adaptor="@openfn/language-gmail@3.2.0", + prompt="Email a summary of state.failed to data-team@example.org with subject 'Nightly sync failures'.", + expect=[r"\bbody\s*:"], + forbid=[r"\btext\s*:", r"\bmessage\s*:", r"\bhtml\s*:", r"\bcontent\s*:"], + doc_ref="Interfaces > SendMessageOptions > body", + why="The reported bug. Nodemailer/SendGrid priors supply `text` or `message`.", + ), + Case( + id="iface.gmail-attachment-filename", + group="interfaces", + adaptor="@openfn/language-gmail@3.2.0", + prompt="Send report.csv to ops@example.org, attaching the CSV string in state.csv.", + expect=[r"filename\s*:"], + # `forbid` is case-sensitive, so `fileName` here would be a distinct + # (wrong) identifier — but `\bname\s*:` already covers the camelCase + # variant's tail, and listing it separately only invites confusion. + forbid=[r"\bname\s*:", r"\bpath\s*:"], + doc_ref="Interfaces > SendMessageOptions > attachments: Array<{filename, content}>", + why="`name` and `path` are the common conventions in other mail libraries.", + ), + Case( + id="iface.http-query-not-params", + group="interfaces", + adaptor="@openfn/language-http@7.3.2", + prompt="GET /patients with a query string of page=2 and size=50.", + expect=[r"query\s*:"], + forbid=[r"params\s*:", r"searchParams\s*:", r"qs\s*:"], + doc_ref="Interfaces > RequestOptions > query", + why="axios and requests both call this `params`, and 6.x used `params` too.", + ), + Case( + id="iface.http-parseas", + group="interfaces", + adaptor="@openfn/language-http@7.3.2", + prompt="GET /export.csv and keep the response as plain text rather than parsing it as JSON.", + expect=[r"parseAs"], + forbid=[r"responseType\s*:", r"\bformat\s*:", r"\bparse\s*:"], + doc_ref="Interfaces > RequestOptions > parseAs", + why="`responseType` is the axios name for the same concept.", + ), + Case( + id="iface.salesforce-bulk-failonerror", + group="interfaces", + adaptor="@openfn/language-salesforce@9.1.5", + prompt="Bulk insert state.rows as Contact records, but don't abort the whole job if some rows fail.", + expect=[r"failOnError"], + forbid=[r"continueOnError", r"allOrNone", r"ignoreErrors"], + doc_ref="Interfaces > Bulk1Options / Bulk2LoadOptions > failOnError", + why="`allOrNone` is the Salesforce API's own name for this, so the model " + "reaches for the platform term over the adaptor's.", + ), + Case( + id="iface.openmrs-getoptions-pagesize", + group="interfaces", + adaptor="@openfn/language-openmrs@5.4.2", + prompt="Fetch patients 100 at a time.", + expect=[r"pageSize", r"\bmax\s*:"], + forbid=[r"\blimit\s*:", r"\bcount\s*:", r"perPage"], + doc_ref="Interfaces > GetOptions > pageSize, max", + why="`limit` is the near-universal convention for this parameter.", + ), + Case( + id="iface.dhis2-apiversion-option", + group="interfaces", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="Call GET on dataElements but pin this one request to API version 40.", + expect=[r"apiVersion"], + forbid=[r"\bversion\s*:", r"\bapi_version\s*:"], + doc_ref="Interfaces > RequestOptions > apiVersion", + why="Nothing in `get(path, params)` hints that a per-request apiVersion exists.", + ), +] + + +# -------------------------------------------------------------------------- +# D. Namespaces — operations under `## ` on the docs page. Present +# in 31 of 107 adaptors and never injected today. +# -------------------------------------------------------------------------- + +NAMESPACES = [ + Case( + id="ns.dhis2-tracker-import", + group="namespaces", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="Import the tracked entities in state.payload through the tracker endpoint.", + expect=[r"tracker\.import"], + forbid=[r"create\(\s*['\"]tracker"], + doc_ref="## tracker > tracker.import", + why="tracker.* is the documented route; create('tracker') explicitly throws in 7.x+.", + ), + Case( + id="ns.dhis2-tracker-import-strategy", + group="namespaces", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="Import state.payload into the tracker, creating new records and updating existing ones.", + expect=[r"CREATE_AND_UPDATE"], + doc_ref="## tracker > tracker.import — import(strategy, payload, options); " + "strategy is CREATE | UPDATE | CREATE_AND_UPDATE | DELETE", + why="The strategy is the first positional argument and its allowed values " + "appear only in the namespace section.", + ), + Case( + id="ns.salesforce-bulk2-insert", + group="namespaces", + adaptor="@openfn/language-salesforce@9.1.5", + prompt="Bulk load 50,000 Contact records from state.rows using the Bulk API.", + expect=[r"bulk2\.insert", r"bulk1\.insert"], + forbid=[r"(? bulk2.insert (and ## bulk1)", + why="The old top-level bulk() was split into bulk1/bulk2 namespaces in 9.x.", + ), + Case( + id="ns.dhis2-util-findattributevalue", + group="namespaces", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="Inside an fn block, pull the 'first name' attribute off state.data.", + expect=[r"util\.findAttributeValue"], + doc_ref="## util > util.findAttributeValue", + why="Moved from top level into the util namespace in 7.0.0.", + ), + Case( + id="ns.http-util-uuid", + group="namespaces", + adaptor="@openfn/language-http@7.3.2", + prompt="Generate a UUID to use as an idempotency key on a POST to /orders.", + expect=[r"util\.uuid"], + forbid=[r"crypto\.randomUUID", r"uuidv4", r"require\("], + doc_ref="## util > util.uuid", + why="The adaptor ships a uuid helper; without it the model hand-rolls one " + "or reaches for a Node API that isn't available in the job DSL.", + ), + Case( + id="ns.salesforce-http-request", + group="namespaces", + adaptor="@openfn/language-salesforce@9.1.5", + prompt="Call a custom Apex REST endpoint at /services/apexrest/MyService using the Salesforce session.", + expect=[r"http\.(get|post|request)"], + doc_ref="## http > http.get / http.post / http.request", + why="Without the namespace the model suggests a raw fetch or the http " + "adaptor instead of Salesforce's authenticated http.* passthrough.", + ), +] + + +# -------------------------------------------------------------------------- +# E. Other sections — configuration-schema and README. Neither is on the docs +# page and neither is fetched today. +# -------------------------------------------------------------------------- + +OTHER = [ + Case( + id="other.dhis2-config-apiversion", + group="other", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="Where does the DHIS2 API version come from if I don't pass one per request?", + target="text", + expect=[r"apiVersion"], + forbid=[r"hard.?cod", r"in your job code"], + doc_ref="configuration-schema > properties.apiVersion (credential field)", + why="apiVersion is a credential field, not a job-code concern. The model " + "has never seen the credential schema.", + ), + Case( + id="other.salesforce-config-securitytoken", + group="other", + adaptor="@openfn/language-salesforce@9.1.5", + prompt="My Salesforce credential keeps failing to authenticate from a new IP. What's likely missing?", + target="text", + expect=[r"securityToken", r"security token"], + doc_ref="configuration-schema > properties.securityToken", + why="The fix is a specific named credential field the model cannot see.", + ), + Case( + id="other.gmail-attachment-archive", + group="other", + adaptor="@openfn/language-gmail@3.2.0", + prompt="Pull the CSV out of the zipped attachment on messages with subject 'daily export'.", + expect=[r"archive"], + forbid=[r"unzip", r"jszip", r"require\("], + doc_ref="Interfaces > MessageContent > archive; README > getContentsFromMessages " + "> options.contents > 'Attachment: archived file'", + why="The adaptor unpacks zips natively via type:'archive'. Without that the " + "model suggests an unzip library that isn't available.", + ), +] + + +# -------------------------------------------------------------------------- +# F. Versions — the same question has different right answers per version. +# Each pin is a real breaking change, confirmed against the adaptor's own +# changelog and against published type declarations for both versions. +# -------------------------------------------------------------------------- + +VERSIONS = [ + Case( + id="ver.http6-post-body-option", + group="version", + adaptor="@openfn/language-http@6.5.4", + prompt="POST the records in state.data to /patients as JSON.", + expect=[r"body\s*:"], + doc_ref="http 6.x: post(path, options) with the payload under options.body. " + "Changed in 7.0.0 — see changelog 7.0.0 'Updated put, patch and post signatures'", + why="Inverse of fn.http-post-data-positional. On 6.x the `body:` key is " + "correct; a latest-only view of the docs makes it look wrong.", + ), + Case( + id="ver.dhis2-6-findattributevalue-toplevel", + group="version", + adaptor="@openfn/language-dhis2@6.3.4", + prompt="Inside an fn block, pull the 'first name' attribute off state.data.", + expect=[r"findAttributeValue"], + forbid=[r"util\.findAttributeValue"], + doc_ref="dhis2 6.x: findAttributeValue is top-level. Moved to util.* in 7.0.0 " + "— see changelog 7.0.0 'Many non-operation functions have moved to the util. namespace'", + why="Inverse of ns.dhis2-util-findattributevalue. On 6.x the util. prefix is wrong.", + ), + Case( + id="ver.dhis2-8-discover-removed", + group="version", + adaptor="@openfn/language-dhis2@8.2.1", + prompt="I want to use discover() to inspect the schema for dataElements before I post. How do I call it?", + # Asserted as a negative on purpose. "It's gone" has unbounded phrasings + # — removed, isn't available, doesn't have one — and chasing them + # produced two false failures before this rewrite. What actually + # matters is bounded: the model must not emit a discover call. + # Answering in prose with no code at all is a pass. + target="code", + expect=[], + forbid=[r"discover\s*\("], + empty_target_passes=True, + doc_ref="dhis2 changelog 7.0.0: 'The discover() function has been removed.'", + why="The model should say it's gone rather than invent a call signature for it.", + ), + Case( + id="ver.salesforce4-bulk-toplevel", + group="version", + adaptor="@openfn/language-salesforce@4.8.6", + prompt="Bulk load 50,000 Contact records from state.rows using the Bulk API.", + expect=[r"\bbulk\("], + forbid=[r"bulk1\.", r"bulk2\."], + doc_ref="salesforce 4.x: top-level bulk(). Split into bulk1.*/bulk2.* namespaces later.", + why="Inverse of ns.salesforce-bulk2-insert. On 4.x the namespaced calls do not exist.", + ), + Case( + id="ver.gmail2-no-getmessagebyid", + group="version", + adaptor="@openfn/language-gmail@2.1.2", + prompt="Fetch one specific message using the Gmail message id in state.data.messageId.", + expect=[r"getContentsFromMessages"], + forbid=[r"getMessageById"], + doc_ref="gmail: getMessageById was added in 3.1.0. It does not exist in 2.x.", + why="A latest-only view offers a function this version does not have.", + ), +] + + +ALL_CASES: list[Case] = SIGNATURES + FUNCTIONS + INTERFACES + NAMESPACES + OTHER + VERSIONS + +GROUPS = ["signatures", "functions", "interfaces", "namespaces", "other", "version"] diff --git a/services/job_chat/tests/integration/adaptor_knowledge/conftest.py b/services/job_chat/tests/integration/adaptor_knowledge/conftest.py new file mode 100644 index 00000000..698afb38 --- /dev/null +++ b/services/job_chat/tests/integration/adaptor_knowledge/conftest.py @@ -0,0 +1,90 @@ +"""Guard: skip individual cases whose adaptor docs can't be loaded. + +Every case depends on job_chat receiving a real adaptor block. When the docs +pipeline can't supply one, the prompt silently degrades to "The user is using +an OpenFn Adaptor to write the job." and the case fails for a reason that has +nothing to do with what it measures. Skipping is the honest outcome — a red +test should mean the model got the API wrong, not that the fixture was empty. + +The check is per adaptor version, not per session, because the version-pinned +cases in the `version` group deliberately reference old releases that a given +machine may not have. + +Known local cause: jsdoc doesn't run under bun (JSDOC_BUN_ERROR.md), so +adaptor_apis can't generate docs and the auto-load path fails. See the README +for the seeding workaround. +""" + +import pytest +from util import AdaptorSpecifier, get_db_connection + + +def _version_present(conn, spec: str) -> bool: + adaptor = AdaptorSpecifier(spec) + with conn.cursor() as cur: + cur.execute( + "SELECT 1 FROM adaptor_function_docs " + "WHERE adaptor_name = %s AND version = %s LIMIT 1", + (adaptor.name, adaptor.version), + ) + return cur.fetchone() is not None + + +@pytest.fixture(scope="session") +def loaded_adaptor_versions(): + """Set of adaptor specifiers that already have docs in the database. + + Empty set (rather than a skip) when the database is unreachable — the + per-case fixture turns that into a skip with a clearer message. + """ + try: + conn = get_db_connection() + except Exception as e: + print(f"\nadaptor_knowledge: no database connection ({e}); all cases will skip") + return set() + + present = set() + try: + # Import here so a collection-time import cycle can't break the fixture. + from .cases import ALL_CASES # noqa: PLC0415 + + for spec in sorted({c.adaptor for c in ALL_CASES}): + try: + if _version_present(conn, spec): + present.add(spec) + except Exception: + break + finally: + conn.close() + + return present + + +@pytest.fixture +def require_adaptor_docs(loaded_adaptor_versions): + """Skip a case unless its exact adaptor version has docs available. + + Returns a callable so the test can pass its own specifier in. Auto-loading + is left to job_chat itself (`download_adaptor_docs` defaults to true); this + only catches the case where the docs are absent AND can't be generated. + """ + + def _check(spec: str) -> None: + if spec in loaded_adaptor_versions: + return + pytest.skip( + f"no adaptor docs for {spec}. job_chat auto-loads on first use where " + f"adaptor_apis works; if it doesn't on this machine, see the README " + f"in this directory for the seeding workaround.", + ) + + return _check + + +def pytest_sessionfinish(session, exitstatus): # noqa: ARG001 + """Print the per-group scoreboard. Silent unless these cases ran.""" + from .scoreboard import render # noqa: PLC0415 + + out = render() + if out: + print(out) diff --git a/services/job_chat/tests/integration/adaptor_knowledge/scoreboard.py b/services/job_chat/tests/integration/adaptor_knowledge/scoreboard.py new file mode 100644 index 00000000..2e9c74ce --- /dev/null +++ b/services/job_chat/tests/integration/adaptor_knowledge/scoreboard.py @@ -0,0 +1,39 @@ +"""Shared result tally for the adaptor-knowledge run. + +Lives in its own module because the test file records into it while the +`pytest_sessionfinish` hook that prints it must live in conftest.py — pytest +does not call session hooks defined in test modules. +""" + +RESULTS: list[tuple[str, str, bool]] = [] +"""One entry per case run, as (case_id, group, passed).""" + + +def record(case_id: str, group: str, passed: bool) -> None: + RESULTS.append((case_id, group, passed)) + + +def render() -> str: + """Per-group scoreboard. Empty string when nothing ran.""" + if not RESULTS: + return "" + + by_group: dict[str, list[bool]] = {} + for _, group, ok in RESULTS: + by_group.setdefault(group, []).append(ok) + + width = max(len(g) for g in by_group) + 2 + lines = ["", "=== Adaptor knowledge ==="] + for group, oks in by_group.items(): + lines.append(f" {group:<{width}}{sum(oks)}/{len(oks)} pass") + + passed = sum(1 for _, _, ok in RESULTS if ok) + lines.append(f" {'TOTAL':<{width}}{passed}/{len(RESULTS)} pass") + + failed = [cid for cid, _, ok in RESULTS if not ok] + if failed: + lines.append("") + lines.append(" failing:") + lines.extend(f" x {cid}" for cid in failed) + + return "\n".join(lines) diff --git a/services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py b/services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py new file mode 100644 index 00000000..249d1b4f --- /dev/null +++ b/services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py @@ -0,0 +1,240 @@ +"""Dev workaround: populate adaptor_function_docs without running jsdoc. + +Only needed on machines where `adaptor_apis` can't generate docs — notably +macOS under bun, where jsdoc dies on `Module.wrapper` (JSDOC_BUN_ERROR.md). +Where the normal pipeline works, job_chat auto-loads on first use and you do +not need this at all. + +It reads the pre-built doclet feed that `embed_docsite` already consumes +(OpenFn/adaptors@docs:docs/docs.json), which contains the same jsdoc output +`adaptor_apis` produces, then pushes it through the real ingest functions +(`filter_function_docs` + `upload_to_postgres`). The resulting rows are what +the live pipeline would have written. + +The feed only carries each adaptor's LATEST version, so older pins fall back +to the package's published `ast.json` on unpkg. Namespaced operations, which +the feed's doclets don't reliably include, are recovered from the rendered +docs page. + +Versions already in the table are left alone — the real pipeline writes richer +rows than these public sources can reconstruct, and `upload_to_postgres` +deletes before inserting. Pass `--force` only when you mean to replace them. + +Run it from the repo root with `services/` on the path, the same root +`entry.py` gives services: + + PYTHONPATH=services poetry run python \\ + services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py +""" + +import json +import re +import sys +import urllib.request +from pathlib import Path + +from dotenv import load_dotenv +from load_adaptor_docs.load_adaptor_docs import ( + create_table_if_not_exists, + filter_function_docs, + upload_to_postgres, +) +from util import AdaptorSpecifier, get_db_connection + +# Same file the repo-root conftest loads, so POSTGRES_URL resolves the same way +# whether this runs standalone or under pytest. +load_dotenv(Path(__file__).resolve().parents[4] / ".env", override=False) + +FEED = "https://raw.githubusercontent.com/OpenFn/adaptors/docs/docs/docs.json" + + +def load_feed(): + with urllib.request.urlopen(FEED, timeout=120) as r: + return json.load(r) + + +def _type_names(t): + """Flatten a doctrine-style type node into jsdoc's `type.names` list.""" + if not t: + return [] + kind = t.get("type") + if kind == "NameExpression": + return [t["name"]] + if kind in ("UnionType", "TypeUnion"): + return [n for e in t.get("elements", []) for n in _type_names(e)] + if kind == "TypeApplication": + base = _type_names(t.get("expression")) + args = [n for a in t.get("applications", []) for n in _type_names(a)] + return [f"{base[0]}.<{','.join(args)}>"] if base else args + if kind in ("OptionalType", "NullableType", "NonNullableType", "RestType"): + return _type_names(t.get("expression")) + return [] + + +def _ast_entry_to_doclet(entry): + """Reshape one ast.json operation into the jsdoc doclet shape ingest expects.""" + tags = (entry.get("docs") or {}).get("tags") or [] + params = [t for t in tags if t.get("title") == "param"] + return { + "kind": "function", + "access": "public", + "scope": "global", + "name": entry["name"], + "signature": f"{entry['name']}({', '.join(entry.get('params') or [])})", + "description": (entry.get("docs") or {}).get("description", ""), + "params": [ + { + "name": p.get("name"), + "type": {"names": _type_names(p.get("type"))}, + "optional": bool(p.get("optional")), + "description": p.get("description") or "", + } + for p in params + ], + "returns": [ + {"type": {"names": _type_names(t.get("type"))}} + for t in tags + if t.get("title") == "returns" + ], + "examples": [t.get("description") for t in tags if t.get("title") == "example"], + } + + +def namespace_doclets(docs_markdown): + """Recover namespaced operations from the rendered docs page. + + The feed's doclet list is inconsistent about namespaces — it carries + dhis2's `tracker.*` and `util.*` but not salesforce's `bulk1.*`/`bulk2.*`. + The rendered page always has them, as `### bulk2.insert` followed by a + `insert(sObject, records, [options]) ...` line, which is + exactly the (function_name, signature) pair the prompt injects. + """ + text = docs_markdown.encode().decode("unicode_escape") + doclets = [] + for section in re.split(r"^##\s+", text, flags=re.M)[1:]: + heading = section.splitlines()[0].strip() + if heading.lower() in ("functions", "interfaces"): + continue + for block in re.split(r"^###\s+", section, flags=re.M)[1:]: + name = block.splitlines()[0].split("{")[0].strip() + if "." not in name: + continue + scope, _, bare = name.partition(".") + sig = re.search(r"([^<]+?)", block) + if not sig: + continue + doclets.append({ + "kind": "function", + "access": "public", + "scope": scope, + "name": bare, + "signature": sig.group(1).split("⇒")[0].strip(), + "description": "", + "params": [], + "returns": [], + "examples": [], + }) + return doclets + + +def load_ast(spec): + """Per-version fallback: the package's published ast.json, from the CDN. + + The docs feed only carries each adaptor's latest version, so this is the + only way to seed the old pins the `version` group needs. + + Fidelity note: ast.json lists top-level operations and the re-exported + common helpers, but NOT namespaced ones (`util.*`, `tracker.*`, `bulk1.*`). + For the versions we need it for, that's faithful — those namespaces did not + exist yet. Don't reach for it to seed a modern version. + """ + pkg, version = spec.rsplit("@", 1) + url = f"https://unpkg.com/{pkg}@{version}/ast.json" + try: + with urllib.request.urlopen(url, timeout=60) as r: + ast = json.load(r) + except Exception as e: + print(f"skip {spec} — no ast.json on the CDN ({e})") + return None + + entries = (ast.get("operations") or []) + (ast.get("common") or []) + return [_ast_entry_to_doclet(e) for e in entries] or None + + +def pair_up(feed): + """The feed alternates {metadata dict} then [jsdoc doclets]. + + Doclets are supplemented with namespaced operations recovered from the + rendered page, which the doclet list doesn't reliably include. + """ + pairs = {} + i = 0 + while i < len(feed): + if isinstance(feed[i], dict): + meta = feed[i] + doclets = feed[i + 1] if i + 1 < len(feed) and isinstance(feed[i + 1], list) else None + if doclets is not None: + have = { + f"{d.get('scope')}.{d.get('name')}" + for d in doclets + if isinstance(d, dict) and d.get("scope") not in (None, "global") + } + extra = [ + d for d in namespace_doclets(meta.get("docs", "")) + if f"{d['scope']}.{d['name']}" not in have + ] + pairs[f"{meta['adaptor']}@{meta['version']}"] = doclets + extra + i += 2 + continue + i += 1 + return pairs + + +def existing_count(conn, spec): + adaptor = AdaptorSpecifier(spec) + with conn.cursor() as cur: + cur.execute( + "SELECT count(*) FROM adaptor_function_docs WHERE adaptor_name = %s AND version = %s", + (adaptor.name, adaptor.version), + ) + return cur.fetchone()[0] + + +def main(wanted, force=False): + """Seed any wanted version that isn't already present. + + Existing rows are left alone unless `force`. `upload_to_postgres` deletes + before inserting, so re-seeding a version that the real pipeline populated + would replace richer rows (namespaced functions, full param docs) with + whatever these public sources can reconstruct. Don't. + """ + feed = pair_up(load_feed()) + conn = get_db_connection() + try: + create_table_if_not_exists(conn) + for spec in wanted: + present = existing_count(conn, spec) + if present and not force: + print(f"have {spec} — {present} functions already, leaving it alone") + continue + + doclets, source = feed.get(spec), "feed" + if doclets is None: + doclets, source = load_ast(spec), "ast.json" + if doclets is None: + continue + + adaptor = AdaptorSpecifier(spec) + docs = filter_function_docs(doclets) + upload_to_postgres(adaptor, docs, conn) + print(f"seed {spec} — {len(docs)} functions (via {source})") + finally: + conn.close() + + +if __name__ == "__main__": + from job_chat.tests.integration.adaptor_knowledge.cases import ALL_CASES + + argv = [a for a in sys.argv[1:] if a != "--force"] + targets = argv or sorted({c.adaptor for c in ALL_CASES}) + main(targets, force="--force" in sys.argv[1:]) diff --git a/services/job_chat/tests/integration/adaptor_knowledge/test_adaptor_knowledge.py b/services/job_chat/tests/integration/adaptor_knowledge/test_adaptor_knowledge.py new file mode 100644 index 00000000..652cca5e --- /dev/null +++ b/services/job_chat/tests/integration/adaptor_knowledge/test_adaptor_knowledge.py @@ -0,0 +1,103 @@ +"""Runs the adaptor-knowledge cases against job_chat and asserts with regexes. + +One pytest item per case. A failure names the doc location the answer should +have come from, so the output doubles as a worklist. + + poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s + poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s -k interfaces + +Every case costs one job_chat call against the live Anthropic API. +""" + +import re + +import pytest +from testing.apollo_client import ApolloClient + +from .cases import ALL_CASES, Case +from .scoreboard import record + + +def _fenced_blocks(text: str) -> str: + """Concatenate the contents of every ``` fence in text.""" + return "\n".join(re.findall(r"```(?:\w+)?\s*\n(.*?)```", text or "", re.DOTALL)) + + +def _haystack(response: dict, target: str) -> str: + """The text a case's regexes run against. + + "code" narrows to generated code — the suggested_code field plus any fenced + blocks in the reply — so that prose *discussing* a wrong key ("you might + expect text:, but...") cannot trip a `forbid`. + """ + reply = response.get("response") or "" + if target == "text": + return reply + return "\n".join([response.get("suggested_code") or "", _fenced_blocks(reply)]) + + +def _build_payload(case: Case) -> dict: + context: dict = {"adaptor": case.adaptor} + if case.expression is not None: + context["expression"] = case.expression + return { + "content": case.prompt, + "context": context, + "suggest_code": True, + "meta": {"session_id": f"sess-adaptor-knowledge-{case.id}"}, + } + + +@pytest.mark.parametrize("case", ALL_CASES, ids=lambda c: c.id) +def test_adaptor_knowledge(case: Case, require_adaptor_docs): + require_adaptor_docs(case.adaptor) + response = ApolloClient().call("job_chat", _build_payload(case)) + hay = _haystack(response, case.target) + + if not hay.strip(): + # Abstention cases assert only that something bad ISN'T generated, so + # generating nothing is a pass. Everywhere else an empty haystack means + # there was nothing to check and the result would be meaningless. + if case.empty_target_passes: + record(case.id, case.group, True) + return + record(case.id, case.group, False) + pytest.fail( + f"{case.id}: no {case.target} in the response to assert on.\n" + f" reply: {(response.get('response') or '')[:300]!r}", + ) + + # `expect` is case-insensitive and satisfied by ANY match: it asks whether a + # concept is present, so it should be generous about how it's spelled. + # + # `forbid` is case-SENSITIVE, because it names a specific wrong identifier. + # Matching loosely there is actively harmful — `fileName` would match the + # correct `filename` under re.I and fail a passing case. + missing = [p for p in case.expect if not re.search(p, hay, re.I)] if case.expect else [] + expect_failed = bool(case.expect) and len(missing) == len(case.expect) + hits = [p for p in case.forbid if re.search(p, hay)] + + passed = not expect_failed and not hits + record(case.id, case.group, passed) + + if passed: + return + + problems = [] + if expect_failed: + problems.append(f"none of the expected patterns matched: {case.expect}") + for p in hits: + problems.append(f"forbidden pattern matched: {p!r}") + + pytest.fail( + "\n".join( + [ + f"{case.id} [{case.group}] {case.adaptor}", + *(f" - {p}" for p in problems), + f" documented at: {case.doc_ref}", + f" expected failure mode: {case.why}", + f" --- {case.target} ---", + hay[:1200], + ], + ), + ) From 60dccb4b03835b5734654e6f857da225f870cd54 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 20 Aug 2026 19:36:04 +0100 Subject: [PATCH 2/5] drop fixture seeding from adaptor knowledge tests --- .../integration/adaptor_knowledge/README.md | 88 +++---- .../integration/adaptor_knowledge/conftest.py | 87 +------ .../adaptor_knowledge/seed_docs.py | 240 ------------------ .../test_adaptor_knowledge.py | 7 +- 4 files changed, 45 insertions(+), 377 deletions(-) delete mode 100644 services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py diff --git a/services/job_chat/tests/integration/adaptor_knowledge/README.md b/services/job_chat/tests/integration/adaptor_knowledge/README.md index 9b55bced..cb3fe36a 100644 --- a/services/job_chat/tests/integration/adaptor_knowledge/README.md +++ b/services/job_chat/tests/integration/adaptor_knowledge/README.md @@ -10,25 +10,21 @@ measures nothing. ## Baseline -Measured on 2026-08-20, `job_chat` unchanged, docs seeded as described below: +**Not yet established.** The numbers this suite produced during development came +from a machine where the docs pipeline couldn't run, on hand-built fixture rows, +so they don't describe the real service and have been removed rather than left +here to be trusted. -| Group | Pass | -|---|---| -| `signatures` (controls) | 3/3 | -| `functions` | 4/6 | -| `interfaces` | 4/7 | -| `namespaces` | **1/6** | -| `other` | 2/3 | -| `version` | 2/2 run (3 skipped, old versions unavailable locally) | -| **Total** | **16/27** | - -Treat this as approximate. The model is stochastic and cases near the boundary -flip between runs — `iface.salesforce-bulk-failonerror` passed one run and -failed the next. Re-baseline before and after any change rather than comparing -against these numbers directly. - -`namespaces` at 1/6 is the standout, and the cause is concrete: see the last -section. +Set the baseline by running the suite once, unchanged, on a machine where +`adaptor_apis` works, and record the scoreboard it prints. Do that before any +retrieval change, not after. + +Two things to know when you do: + +- The model is stochastic and cases near the boundary flip between identical + runs. A one- or two-point move is noise. Run it more than once. +- `namespaces` was by far the weakest group in every development run, and the + cause is concrete rather than statistical: see the last section. ## Running them @@ -83,8 +79,8 @@ rather than by *what you're testing* is what keeps both harnesses simple. |---|---| | `cases.py` | The 30 cases as data. Edit this to add or tune probes. | | `test_adaptor_knowledge.py` | Parametrized runner, regex assertions, scoreboard. | -| `conftest.py` | Skips a case when its adaptor version has no docs. | -| `seed_docs.py` | Dev workaround for machines where jsdoc can't run. | +| `conftest.py` | Prints the scoreboard at the end of the run. | +| `scoreboard.py` | The tally the runner writes and conftest prints. | ## The groups @@ -97,8 +93,8 @@ rather than by *what you're testing* is what keeps both harnesses simple. | `other` | 3 | `configuration-schema` and the README | | `version` | 5 | Behaviour that differs between two pinned versions | -The `signatures` group is the baseline. If those fail, something is wrong with -the fixture rather than with retrieval. +The `signatures` group is the baseline. If those fail, job_chat isn't getting +an adaptor block at all and no other number in the run means anything. Four `version` cases are deliberate inverses of a case in another group — the same question, a different pin, and the opposite correct answer: @@ -107,7 +103,7 @@ same question, a different pin, and the opposite correct answer: |---|---| | `fn.http-post-data-positional` (7.3.2, `post(path, data)`) | `ver.http6-post-body-option` (6.5.4, `{body: ...}`) | | `ns.dhis2-util-findattributevalue` (8.2.1, `util.` prefix) | `ver.dhis2-6-findattributevalue-toplevel` (6.3.4, no prefix) | -| `ns.salesforce-bulk2-insert` (9.1.1, `bulk2.insert`) | `ver.salesforce4-bulk-toplevel` (4.8.6, `bulk()`) | +| `ns.salesforce-bulk2-insert` (9.1.5, `bulk2.insert`) | `ver.salesforce4-bulk-toplevel` (4.8.6, `bulk()`) | A method that just dumps the latest docs will pass one side of each pair and fail the other. That's the pair's job. @@ -124,39 +120,23 @@ key ("you might reach for `text:`, but...") would otherwise trip a `forbid`. ## Adaptor docs have to be present -A case is only meaningful if job_chat receives a real adaptor block. When the -docs are missing the prompt quietly degrades to "The user is using an OpenFn -Adaptor to write the job.", so `conftest.py` skips those cases rather than -letting them fail for the wrong reason. +A case only measures anything if job_chat receives a real adaptor block. When +the docs are missing the prompt quietly degrades to "The user is using an +OpenFn Adaptor to write the job." and every case fails for a reason that has +nothing to do with retrieval. + +There is no fixture to set up: job_chat auto-loads an adaptor's docs on first +use (`download_adaptor_docs` defaults to true), through the same pipeline +production uses. Run the suite somewhere that pipeline works. -Where `adaptor_apis` works, job_chat auto-loads on first use and there is -nothing to do. Where it doesn't — notably macOS under bun, where jsdoc dies on -`Module.wrapper` (see `JSDOC_BUN_ERROR.md`) — seed the table first: +It does not work on macOS under bun, where jsdoc dies on `Module.wrapper` (see +`JSDOC_BUN_ERROR.md`). A full-red run there is a broken toolchain, not a score. +Check before believing a number: ```bash -poetry run python services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py +psql "$POSTGRES_URL" -c "SELECT adaptor_name, version, count(*) FROM adaptor_function_docs GROUP BY 1,2 ORDER BY 1,2" ``` -That pulls the pre-built doclet feed the docsite indexer already uses and -pushes it through the real ingest functions, so the rows match what the live -pipeline would write. It only covers each adaptor's **latest** version, so the -old pins in the `version` group still skip unless that version is already in -your database. - -## What the failures are telling you - -Two upstream causes account for most of them, both verified in the code: - -1. `job_chat/prompt.py` injects only the `signature` column. The - `function_data` JSONB alongside it already holds descriptions, parameter - docs and examples — nothing reads them. -2. `load_adaptor_docs.filter_function_docs` keeps only `function` / - `external-function` / `external` doclets, so `@typedef` blocks — the only - definition of option-object property names — are never stored. - -There is also a third, narrower one worth fixing on its own: the namespace -prefix is stored in `function_name` (`bulk1.insert`) but not in `signature` -(`insert(sObject, records, options)`), and only the signature is injected. The -salesforce block therefore lists `insert(...)` three times with no way to tell -the variants apart, and shows `get`/`post`/`request` as if they were top-level -when they are `http.*`. +Don't hand-populate that table to get a green-ish run. Rows written from +anything other than the real pipeline make the score unreadable — you no longer +know whether a change moved retrieval or just moved the fixture. diff --git a/services/job_chat/tests/integration/adaptor_knowledge/conftest.py b/services/job_chat/tests/integration/adaptor_knowledge/conftest.py index 698afb38..315c468c 100644 --- a/services/job_chat/tests/integration/adaptor_knowledge/conftest.py +++ b/services/job_chat/tests/integration/adaptor_knowledge/conftest.py @@ -1,90 +1,15 @@ -"""Guard: skip individual cases whose adaptor docs can't be loaded. +"""Prints the scoreboard at the end of the run. -Every case depends on job_chat receiving a real adaptor block. When the docs -pipeline can't supply one, the prompt silently degrades to "The user is using -an OpenFn Adaptor to write the job." and the case fails for a reason that has -nothing to do with what it measures. Skipping is the honest outcome — a red -test should mean the model got the API wrong, not that the fixture was empty. - -The check is per adaptor version, not per session, because the version-pinned -cases in the `version` group deliberately reference old releases that a given -machine may not have. - -Known local cause: jsdoc doesn't run under bun (JSDOC_BUN_ERROR.md), so -adaptor_apis can't generate docs and the auto-load path fails. See the README -for the seeding workaround. +The hook has to live here: pytest does not call session hooks defined in test +modules, and the tally itself is in `scoreboard.py` because the test module +writes to it while this reads it. """ -import pytest -from util import AdaptorSpecifier, get_db_connection - - -def _version_present(conn, spec: str) -> bool: - adaptor = AdaptorSpecifier(spec) - with conn.cursor() as cur: - cur.execute( - "SELECT 1 FROM adaptor_function_docs " - "WHERE adaptor_name = %s AND version = %s LIMIT 1", - (adaptor.name, adaptor.version), - ) - return cur.fetchone() is not None - - -@pytest.fixture(scope="session") -def loaded_adaptor_versions(): - """Set of adaptor specifiers that already have docs in the database. - - Empty set (rather than a skip) when the database is unreachable — the - per-case fixture turns that into a skip with a clearer message. - """ - try: - conn = get_db_connection() - except Exception as e: - print(f"\nadaptor_knowledge: no database connection ({e}); all cases will skip") - return set() - - present = set() - try: - # Import here so a collection-time import cycle can't break the fixture. - from .cases import ALL_CASES # noqa: PLC0415 - - for spec in sorted({c.adaptor for c in ALL_CASES}): - try: - if _version_present(conn, spec): - present.add(spec) - except Exception: - break - finally: - conn.close() - - return present - - -@pytest.fixture -def require_adaptor_docs(loaded_adaptor_versions): - """Skip a case unless its exact adaptor version has docs available. - - Returns a callable so the test can pass its own specifier in. Auto-loading - is left to job_chat itself (`download_adaptor_docs` defaults to true); this - only catches the case where the docs are absent AND can't be generated. - """ - - def _check(spec: str) -> None: - if spec in loaded_adaptor_versions: - return - pytest.skip( - f"no adaptor docs for {spec}. job_chat auto-loads on first use where " - f"adaptor_apis works; if it doesn't on this machine, see the README " - f"in this directory for the seeding workaround.", - ) - - return _check - -def pytest_sessionfinish(session, exitstatus): # noqa: ARG001 +def pytest_sessionfinish(session: object, exitstatus: int) -> None: # noqa: ARG001 """Print the per-group scoreboard. Silent unless these cases ran.""" from .scoreboard import render # noqa: PLC0415 out = render() if out: - print(out) + print(out) # noqa: T201 — the scoreboard is the point of the run diff --git a/services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py b/services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py deleted file mode 100644 index 249d1b4f..00000000 --- a/services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py +++ /dev/null @@ -1,240 +0,0 @@ -"""Dev workaround: populate adaptor_function_docs without running jsdoc. - -Only needed on machines where `adaptor_apis` can't generate docs — notably -macOS under bun, where jsdoc dies on `Module.wrapper` (JSDOC_BUN_ERROR.md). -Where the normal pipeline works, job_chat auto-loads on first use and you do -not need this at all. - -It reads the pre-built doclet feed that `embed_docsite` already consumes -(OpenFn/adaptors@docs:docs/docs.json), which contains the same jsdoc output -`adaptor_apis` produces, then pushes it through the real ingest functions -(`filter_function_docs` + `upload_to_postgres`). The resulting rows are what -the live pipeline would have written. - -The feed only carries each adaptor's LATEST version, so older pins fall back -to the package's published `ast.json` on unpkg. Namespaced operations, which -the feed's doclets don't reliably include, are recovered from the rendered -docs page. - -Versions already in the table are left alone — the real pipeline writes richer -rows than these public sources can reconstruct, and `upload_to_postgres` -deletes before inserting. Pass `--force` only when you mean to replace them. - -Run it from the repo root with `services/` on the path, the same root -`entry.py` gives services: - - PYTHONPATH=services poetry run python \\ - services/job_chat/tests/integration/adaptor_knowledge/seed_docs.py -""" - -import json -import re -import sys -import urllib.request -from pathlib import Path - -from dotenv import load_dotenv -from load_adaptor_docs.load_adaptor_docs import ( - create_table_if_not_exists, - filter_function_docs, - upload_to_postgres, -) -from util import AdaptorSpecifier, get_db_connection - -# Same file the repo-root conftest loads, so POSTGRES_URL resolves the same way -# whether this runs standalone or under pytest. -load_dotenv(Path(__file__).resolve().parents[4] / ".env", override=False) - -FEED = "https://raw.githubusercontent.com/OpenFn/adaptors/docs/docs/docs.json" - - -def load_feed(): - with urllib.request.urlopen(FEED, timeout=120) as r: - return json.load(r) - - -def _type_names(t): - """Flatten a doctrine-style type node into jsdoc's `type.names` list.""" - if not t: - return [] - kind = t.get("type") - if kind == "NameExpression": - return [t["name"]] - if kind in ("UnionType", "TypeUnion"): - return [n for e in t.get("elements", []) for n in _type_names(e)] - if kind == "TypeApplication": - base = _type_names(t.get("expression")) - args = [n for a in t.get("applications", []) for n in _type_names(a)] - return [f"{base[0]}.<{','.join(args)}>"] if base else args - if kind in ("OptionalType", "NullableType", "NonNullableType", "RestType"): - return _type_names(t.get("expression")) - return [] - - -def _ast_entry_to_doclet(entry): - """Reshape one ast.json operation into the jsdoc doclet shape ingest expects.""" - tags = (entry.get("docs") or {}).get("tags") or [] - params = [t for t in tags if t.get("title") == "param"] - return { - "kind": "function", - "access": "public", - "scope": "global", - "name": entry["name"], - "signature": f"{entry['name']}({', '.join(entry.get('params') or [])})", - "description": (entry.get("docs") or {}).get("description", ""), - "params": [ - { - "name": p.get("name"), - "type": {"names": _type_names(p.get("type"))}, - "optional": bool(p.get("optional")), - "description": p.get("description") or "", - } - for p in params - ], - "returns": [ - {"type": {"names": _type_names(t.get("type"))}} - for t in tags - if t.get("title") == "returns" - ], - "examples": [t.get("description") for t in tags if t.get("title") == "example"], - } - - -def namespace_doclets(docs_markdown): - """Recover namespaced operations from the rendered docs page. - - The feed's doclet list is inconsistent about namespaces — it carries - dhis2's `tracker.*` and `util.*` but not salesforce's `bulk1.*`/`bulk2.*`. - The rendered page always has them, as `### bulk2.insert` followed by a - `insert(sObject, records, [options]) ...` line, which is - exactly the (function_name, signature) pair the prompt injects. - """ - text = docs_markdown.encode().decode("unicode_escape") - doclets = [] - for section in re.split(r"^##\s+", text, flags=re.M)[1:]: - heading = section.splitlines()[0].strip() - if heading.lower() in ("functions", "interfaces"): - continue - for block in re.split(r"^###\s+", section, flags=re.M)[1:]: - name = block.splitlines()[0].split("{")[0].strip() - if "." not in name: - continue - scope, _, bare = name.partition(".") - sig = re.search(r"([^<]+?)", block) - if not sig: - continue - doclets.append({ - "kind": "function", - "access": "public", - "scope": scope, - "name": bare, - "signature": sig.group(1).split("⇒")[0].strip(), - "description": "", - "params": [], - "returns": [], - "examples": [], - }) - return doclets - - -def load_ast(spec): - """Per-version fallback: the package's published ast.json, from the CDN. - - The docs feed only carries each adaptor's latest version, so this is the - only way to seed the old pins the `version` group needs. - - Fidelity note: ast.json lists top-level operations and the re-exported - common helpers, but NOT namespaced ones (`util.*`, `tracker.*`, `bulk1.*`). - For the versions we need it for, that's faithful — those namespaces did not - exist yet. Don't reach for it to seed a modern version. - """ - pkg, version = spec.rsplit("@", 1) - url = f"https://unpkg.com/{pkg}@{version}/ast.json" - try: - with urllib.request.urlopen(url, timeout=60) as r: - ast = json.load(r) - except Exception as e: - print(f"skip {spec} — no ast.json on the CDN ({e})") - return None - - entries = (ast.get("operations") or []) + (ast.get("common") or []) - return [_ast_entry_to_doclet(e) for e in entries] or None - - -def pair_up(feed): - """The feed alternates {metadata dict} then [jsdoc doclets]. - - Doclets are supplemented with namespaced operations recovered from the - rendered page, which the doclet list doesn't reliably include. - """ - pairs = {} - i = 0 - while i < len(feed): - if isinstance(feed[i], dict): - meta = feed[i] - doclets = feed[i + 1] if i + 1 < len(feed) and isinstance(feed[i + 1], list) else None - if doclets is not None: - have = { - f"{d.get('scope')}.{d.get('name')}" - for d in doclets - if isinstance(d, dict) and d.get("scope") not in (None, "global") - } - extra = [ - d for d in namespace_doclets(meta.get("docs", "")) - if f"{d['scope']}.{d['name']}" not in have - ] - pairs[f"{meta['adaptor']}@{meta['version']}"] = doclets + extra - i += 2 - continue - i += 1 - return pairs - - -def existing_count(conn, spec): - adaptor = AdaptorSpecifier(spec) - with conn.cursor() as cur: - cur.execute( - "SELECT count(*) FROM adaptor_function_docs WHERE adaptor_name = %s AND version = %s", - (adaptor.name, adaptor.version), - ) - return cur.fetchone()[0] - - -def main(wanted, force=False): - """Seed any wanted version that isn't already present. - - Existing rows are left alone unless `force`. `upload_to_postgres` deletes - before inserting, so re-seeding a version that the real pipeline populated - would replace richer rows (namespaced functions, full param docs) with - whatever these public sources can reconstruct. Don't. - """ - feed = pair_up(load_feed()) - conn = get_db_connection() - try: - create_table_if_not_exists(conn) - for spec in wanted: - present = existing_count(conn, spec) - if present and not force: - print(f"have {spec} — {present} functions already, leaving it alone") - continue - - doclets, source = feed.get(spec), "feed" - if doclets is None: - doclets, source = load_ast(spec), "ast.json" - if doclets is None: - continue - - adaptor = AdaptorSpecifier(spec) - docs = filter_function_docs(doclets) - upload_to_postgres(adaptor, docs, conn) - print(f"seed {spec} — {len(docs)} functions (via {source})") - finally: - conn.close() - - -if __name__ == "__main__": - from job_chat.tests.integration.adaptor_knowledge.cases import ALL_CASES - - argv = [a for a in sys.argv[1:] if a != "--force"] - targets = argv or sorted({c.adaptor for c in ALL_CASES}) - main(targets, force="--force" in sys.argv[1:]) diff --git a/services/job_chat/tests/integration/adaptor_knowledge/test_adaptor_knowledge.py b/services/job_chat/tests/integration/adaptor_knowledge/test_adaptor_knowledge.py index 652cca5e..c5a7a64e 100644 --- a/services/job_chat/tests/integration/adaptor_knowledge/test_adaptor_knowledge.py +++ b/services/job_chat/tests/integration/adaptor_knowledge/test_adaptor_knowledge.py @@ -7,6 +7,10 @@ poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s -k interfaces Every case costs one job_chat call against the live Anthropic API. + +job_chat loads the adaptor's docs itself on first use, so there is no fixture +to set up. If those docs can't be generated on this machine the prompt has no +adaptor block and the cases fail — that is a broken environment, not a score. """ import re @@ -49,8 +53,7 @@ def _build_payload(case: Case) -> dict: @pytest.mark.parametrize("case", ALL_CASES, ids=lambda c: c.id) -def test_adaptor_knowledge(case: Case, require_adaptor_docs): - require_adaptor_docs(case.adaptor) +def test_adaptor_knowledge(case: Case) -> None: response = ApolloClient().call("job_chat", _build_payload(case)) hay = _haystack(response, case.target) From 7c791517b545ec94c9a9b6e4e0d49b6358503a9f Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 20 Aug 2026 19:44:20 +0100 Subject: [PATCH 3/5] fix three misjudging assertions, drop scratch repro spec --- .../bugs/test_repro_gmail_sendmessage_keys.md | 99 ------------------- .../integration/adaptor_knowledge/cases.py | 24 +++-- 2 files changed, 17 insertions(+), 106 deletions(-) delete mode 100644 services/job_chat/tests/acceptance/bugs/test_repro_gmail_sendmessage_keys.md diff --git a/services/job_chat/tests/acceptance/bugs/test_repro_gmail_sendmessage_keys.md b/services/job_chat/tests/acceptance/bugs/test_repro_gmail_sendmessage_keys.md deleted file mode 100644 index e7f6fc39..00000000 --- a/services/job_chat/tests/acceptance/bugs/test_repro_gmail_sendmessage_keys.md +++ /dev/null @@ -1,99 +0,0 @@ ---- -id: job-chat.tmp.repro-gmail-sendmessage-keys -service: job_chat -runs: 5 -judges: [general, openfn_code_quality] ---- - -# notes - -Reproduction of a user report: when asked to send mail with the gmail adaptor, -job_chat writes `sendMessage({ to, subject, text: ... })` or -`sendMessage({ to, subject, message: ... })`. Neither key exists on -`SendMessageOptions` — the body key is `body` — so the operation sends an empty -message or throws at run time. The reporter saw it "at least half a dozen -times", hence `runs: 5`: this measures a rate, not a single verdict. - -Why it happens (not a judgement call — verified in the code): - -- `job_chat/prompt.py:generate_system_message` injects ONLY the `signature` - column from `adaptor_function_docs`. For gmail@3.2.0 the entire adaptor - context the model receives is a bare name list, ending in - `sendMessage(message)`. The stored `function_data` JSONB does hold the - param descriptions and the docsite example (which uses `body`), but nothing - reads them. -- `load_adaptor_docs.filter_function_docs` keeps only doclets of kind - `function` / `external-function` / `external`, so the `SendMessageOptions` - typedef — the only place the `body`/`to`/`subject`/`attachments` property - names are defined — is never stored at all. -- `job_chat/retrieve_docs.py:search_docs` pins the docsite RAG to - `docs_type="general_docs"`, so the adaptor docs page can't fill the gap - either. The prompt even says so: "not adaptor-specific APIs, which are - included separately." - -So `body` appears nowhere in the prompt, and the model falls back on its -nodemailer / SendGrid priors, where the body key IS `text` (or `html`, or -`message`). The user's complaint that it ignores "the adaptor doc" is -accurate about docs.openfn.org, but that document never reaches the model. - -Note for local runs: gmail docs must be present in `adaptor_function_docs`, or -the adaptor block degrades to "The user is using an OpenFn Adaptor to write the -job." and the test is no longer a faithful repro. Confirm with -`select signature from adaptor_function_docs where adaptor_name = '@openfn/language-gmail'`. - -Expected behaviour once fixed: the message object uses `body`, and no invented -key. A model that cannot know the key names should say so or ask, not guess -silently. - -# quality_criteria - -- Any `sendMessage` call passes the body text under the key `body`. -- The message object uses no invented key for the body — specifically NOT `text`, `message`, `html`, `content`, or `bodyText`. -- Recipient and subject use the documented keys `to` and `subject`. -- The code calls only functions that exist in the gmail adaptor (`sendMessage`, `getContentsFromMessages`, `getMessageById`) or in language-common; it does not invent a mail-sending function such as `send`, `sendEmail`, or `sendMail`. -- The response does not claim the adaptor supports message fields it has not been shown; if it is unsure of the option names it says so or asks, rather than presenting a guessed key as documented. - -# settings - -## context.expression - -```js -fn(state => { - const failed = state.data.filter(r => r.status === 'error'); - return { ...state, failed }; -}); -``` - -## context.adaptor - -@openfn/language-gmail@3.2.0 - -## context.input - -```json -{ - "data": [ - { "id": "r-1001", "patient": "P-88", "status": "ok" }, - { "id": "r-1002", "patient": "P-91", "status": "error", "reason": "missing dob" }, - { "id": "r-1003", "patient": "P-92", "status": "error", "reason": "bad org unit" } - ] -} -``` - -## suggest_code - -true - -## meta.session_id - -sess-tmp-repro-gmail-sendmessage-keys-0001 - -# turn - -## role - -user - -## content - -now email the failed records to data-team@example.org as a summary, subject "Nightly sync failures" diff --git a/services/job_chat/tests/integration/adaptor_knowledge/cases.py b/services/job_chat/tests/integration/adaptor_knowledge/cases.py index bbf43f20..a7cb6cab 100644 --- a/services/job_chat/tests/integration/adaptor_knowledge/cases.py +++ b/services/job_chat/tests/integration/adaptor_knowledge/cases.py @@ -85,8 +85,11 @@ class Case: adaptor="@openfn/language-http@7.3.2", prompt="I need to send a HEAD request. Which function supports an arbitrary HTTP method?", target="text", - expect=[r"\brequest\b"], - forbid=[r"\bhead\("], + # NOT bare `request` — the word is in the question and in any prose about + # HTTP, so it matched answers that never named the function. Require it + # in a form that identifies it as the function being recommended. + expect=[r"request\s*\(", r"`request`"], + forbid=[r"\bhead\(", r"method\s*:\s*['\"]HEAD"], doc_ref="Functions > request (in the injected signature list)", why="Control: request(method, path, options) is in the signature list.", ), @@ -126,11 +129,15 @@ class Case: group="functions", adaptor="@openfn/language-salesforce@9.1.5", prompt="Create three Contact records from state.contacts in a single call.", - expect=[r"create\("], - forbid=[r"each\s*\("], + # Assert the documented fact directly: the collection goes in as the + # second argument. Looping with each() fails this because the second + # argument becomes the per-item cursor instead — no need to forbid + # each() by name, which would have been a style judgement rather than + # anything the docs state. + expect=[r"create\(\s*['\"]Contact['\"]\s*,\s*(\$\.|state\.)contacts"], doc_ref="Functions > create — create(sObjectName, records); records is an Array", why="The signature says `records` but not that it accepts an array, so the " - "model reaches for each() to loop instead of one bulk-ish call.", + "model loops instead of passing the collection straight in.", ), Case( id="fn.gmail-getcontents-query-key", @@ -335,8 +342,11 @@ class Case: adaptor="@openfn/language-dhis2@8.2.1", prompt="Where does the DHIS2 API version come from if I don't pass one per request?", target="text", - expect=[r"apiVersion"], - forbid=[r"hard.?cod", r"in your job code"], + # The discriminating fact is WHERE the value comes from, so assert that. + # The old forbids (`hard.?cod`, `in your job code`) matched the correct + # answer — "it's a credential field, don't hardcode it in your job code" + # tripped both — and so scored a right answer as wrong. + expect=[r"credential", r"configuration"], doc_ref="configuration-schema > properties.apiVersion (credential field)", why="apiVersion is a credential field, not a job-code concern. The model " "has never seen the credential schema.", From b46fc57f774754807e7c9feb2c8f9e41999592be Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 20 Aug 2026 19:57:40 +0100 Subject: [PATCH 4/5] rewrite adaptor knowledge readme for retrieval work --- .../integration/adaptor_knowledge/README.md | 220 +++++++++--------- 1 file changed, 112 insertions(+), 108 deletions(-) diff --git a/services/job_chat/tests/integration/adaptor_knowledge/README.md b/services/job_chat/tests/integration/adaptor_knowledge/README.md index cb3fe36a..469dcdcd 100644 --- a/services/job_chat/tests/integration/adaptor_knowledge/README.md +++ b/services/job_chat/tests/integration/adaptor_knowledge/README.md @@ -1,103 +1,87 @@ # Adaptor knowledge probes -Thirty cases that ask job_chat something whose correct answer lives in one -specific, named place in an adaptor's documentation, then check the answer with -a regex. They exist to give anyone working on adaptor-docs retrieval a -scoreboard: a number that should go up as the method improves. +A 30-case benchmark for **how well job_chat can answer questions whose answer +lives in an adaptor's documentation**. Each case asks a question with exactly +one documented right answer, then checks the reply with a regex. The run prints +a per-group score. -Many of them fail today. That's the point — a suite that already passes -measures nothing. +Use it to: -## Baseline +- **Measure a new retrieval method.** Score before, change the pipeline, score + after. The groups tell you which part of the docs surface your method reached. +- **Catch regressions.** Anything that changes prompt construction, doc ingest, + or model version can quietly cost adaptor accuracy. Nothing else in the repo + would notice. -**Not yet established.** The numbers this suite produced during development came -from a machine where the docs pipeline couldn't run, on hand-built fixture rows, -so they don't describe the real service and have been removed rather than left -here to be trusted. +Many cases fail on the current pipeline. That's deliberate — a benchmark that +starts green has no room to measure an improvement. -Set the baseline by running the suite once, unchanged, on a machine where -`adaptor_apis` works, and record the scoreboard it prints. Do that before any -retrieval change, not after. +## Before you run: check the docs pipeline -Two things to know when you do: +job_chat loads an adaptor's docs by calling the `adaptor_apis` service over HTTP +on port 3000, which fetches adaptor source from the GitHub contents API. If that +chain is broken the prompt degrades *silently* — no error, just a generic +"The user is using an OpenFn Adaptor" line — and every case fails for a reason +that has nothing to do with retrieval. -- The model is stochastic and cases near the boundary flip between identical - runs. A one- or two-point move is noise. Run it more than once. -- `namespaces` was by far the weakest group in every development run, and the - cause is concrete rather than statistical: see the last section. - -## Running them +So: start the server, then preflight it. ```bash -poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s +bun start ``` -One group at a time: - ```bash -poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s -k interfaces +curl -s -X POST http://127.0.0.1:3000/services/adaptor_apis -H 'Content-Type: application/json' -d '{"adaptors":["@openfn/language-gmail@3.2.0"]}' ``` -Each case costs one job_chat call against the live Anthropic API. The run -prints a per-group scoreboard at the end. - -## Why here, and not under `acceptance/` +You want `errors: []` and a non-empty `docs`. If you get +`{"docs":{},"errors":[...]}`, note that **the reason is not in that response** — +it's discarded before it reaches any caller, and only printed to the server's +own console. Go read the terminal running `bun start`. -Three reasons, in order of weight: +Don't hand-populate `adaptor_function_docs` to get past a failing preflight. +Rows from anywhere but the real pipeline make the score unreadable: you can no +longer tell whether a change moved retrieval or just moved the fixture. -**These are pass/fail, not judged.** Every assertion is a regex over the -response. Nobody has to read 30 verdicts to learn what happened — you read one -scoreboard line per group. The `acceptance/` tier is built around -`spec_collector` turning markdown specs into LLM-judged items, which is the -right tool when "is this answer good?" needs judgement, and the wrong one when -the question is "does the string `body:` appear in the generated code?". +## Running -**The tier marker follows the directory.** The repo-root `conftest.py` applies -`unit` / `service` / `integration` / `acceptance` based on which of those names -appears in the test's path. These tests hit a live LLM and Postgres, which is -the repo's own definition of `integration` ("hits real external services... -Manual/nightly"). Putting them under `integration/` gets the correct marker -with no new machinery. +```bash +poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s +``` -**Excluding them is automatic.** They aren't in an `acceptance/` directory, so -`spec_collector` never collects them and an acceptance run never touches them. -Nothing to remember, no flag to pass. +One group at a time: -On the nesting question: a topic folder *inside* a tier folder is fine and is -what the marker logic expects. The thing to avoid is the inverse — tier folders -nested under a topic folder — which would still technically work (the root -conftest matches any path segment) but reads backwards. +```bash +poetry run pytest services/job_chat/tests/integration/adaptor_knowledge -s -k namespaces +``` -If a probe ever needs real judgement rather than a string match, it belongs in -`../../acceptance/` as a markdown spec, alongside -`bugs/test_repro_gmail_sendmessage_keys.md`. Splitting by *how you assert* -rather than by *what you're testing* is what keeps both harnesses simple. +`-s` is required — the scoreboard prints on stdout. Each case is one live +Anthropic call, so a full run costs 30. -## Layout +These carry the `integration` marker (from the directory name) and live outside +`acceptance/`, so a normal acceptance run never collects them. -| File | What it is | -|---|---| -| `cases.py` | The 30 cases as data. Edit this to add or tune probes. | -| `test_adaptor_knowledge.py` | Parametrized runner, regex assertions, scoreboard. | -| `conftest.py` | Prints the scoreboard at the end of the run. | -| `scoreboard.py` | The tally the runner writes and conftest prints. | +## Reading the score -## The groups +The groups are not arbitrary. Each one names a **distinct capability** a +retrieval method has to have, and the doc surface it has to reach: -| Group | n | Doc location being probed | -|---|---|---| -| `signatures` | 3 | The function list job_chat already injects. **Controls — these should pass.** | -| `functions` | 6 | `## Functions` — parameter names, order, examples | -| `interfaces` | 7 | `## Interfaces` — `@typedef` property names | -| `namespaces` | 6 | `## ` — `tracker.*`, `bulk1/2.*`, `util.*`, `http.*` | -| `other` | 3 | `configuration-schema` and the README | -| `version` | 5 | Behaviour that differs between two pinned versions | +| Group | n | What passing it proves | Where that lives | +|---|---|---|---| +| `signatures` | 3 | Control. The function list job_chat already injects. | already in the prompt | +| `functions` | 6 | You retrieve parameter *semantics* — order, meaning, examples — not just the signature line | `## Functions` bodies | +| `interfaces` | 7 | You retrieve option-object property names | `## Interfaces` `@typedef`s | +| `namespaces` | 6 | You preserve the namespace prefix (`tracker.*`, `bulk2.*`, `util.*`, `http.*`) | `## ` | +| `other` | 3 | You reach beyond the API docs page | `configuration-schema`, README | +| `version` | 5 | You retrieve for the *pinned* version, not the latest | per-version docs | -The `signatures` group is the baseline. If those fail, job_chat isn't getting -an adaptor block at all and no other number in the run means anything. +**`signatures` is a smoke test, not a score.** If those three fail, job_chat +isn't getting an adaptor block at all — fix the prerequisites above and rerun. +No other number in that run means anything. -Four `version` cases are deliberate inverses of a case in another group — the -same question, a different pin, and the opposite correct answer: +**The `version` group catches the obvious shortcut.** Four of its five cases are +deliberate inverses of a case in another group: identical prompt, different +pinned version, opposite correct answer. | Latest-version case | Old-version inverse | |---|---| @@ -105,38 +89,58 @@ same question, a different pin, and the opposite correct answer: | `ns.dhis2-util-findattributevalue` (8.2.1, `util.` prefix) | `ver.dhis2-6-findattributevalue-toplevel` (6.3.4, no prefix) | | `ns.salesforce-bulk2-insert` (9.1.5, `bulk2.insert`) | `ver.salesforce4-bulk-toplevel` (4.8.6, `bulk()`) | -A method that just dumps the latest docs will pass one side of each pair and -fail the other. That's the pair's job. - -## Adding a case - -Append a `Case` to the right list in `cases.py`. Fill in `doc_ref` with the -exact section the answer comes from, and `why` with the wrong answer you -expect. Both print on failure, which is what makes a red run actionable rather -than just red. - -Keep `target="code"` for anything that asks for code. Prose discussing a wrong -key ("you might reach for `text:`, but...") would otherwise trip a `forbid`. - -## Adaptor docs have to be present - -A case only measures anything if job_chat receives a real adaptor block. When -the docs are missing the prompt quietly degrades to "The user is using an -OpenFn Adaptor to write the job." and every case fails for a reason that has -nothing to do with retrieval. - -There is no fixture to set up: job_chat auto-loads an adaptor's docs on first -use (`download_adaptor_docs` defaults to true), through the same pipeline -production uses. Run the suite somewhere that pipeline works. - -It does not work on macOS under bun, where jsdoc dies on `Module.wrapper` (see -`JSDOC_BUN_ERROR.md`). A full-red run there is a broken toolchain, not a score. -Check before believing a number: - -```bash -psql "$POSTGRES_URL" -c "SELECT adaptor_name, version, count(*) FROM adaptor_function_docs GROUP BY 1,2 ORDER BY 1,2" -``` - -Don't hand-populate that table to get a green-ish run. Rows written from -anything other than the real pipeline make the score unreadable — you no longer -know whether a change moved retrieval or just moved the fixture. +A method that indexes only the latest docs passes one side of each pair and +fails the other, so its total barely moves while it has clearly got worse for +anyone pinned to an older adaptor. Watch the pairs, not just the total. + +**Noise.** The model is stochastic and borderline cases flip between identical +runs. Treat a one- or two-point move as noise; run three times and compare +per-group rates before believing a change helped. + +## Where the information is lost today + +A starting map for anyone about to rewrite this. All three are in the current +pipeline, and each maps to a group above: + +1. **Only the `signature` column is injected.** `prompt.py:generate_system_message` + reads `signature` and ignores `function_data`, which already holds + descriptions, params and examples. → `functions` +2. **`@typedef` doclets are dropped at ingest.** `load_adaptor_docs.filter_function_docs` + discards them, so option-object properties like `SendMessageOptions.body` + never reach the database at all. → `interfaces` +3. **The namespace prefix isn't in the signature.** It's in `function_name` + (`bulk1.insert`) but not in `signature` (`insert(...)`), and only the + signature is injected — so three different `insert(...)` lines appear + identical, and `http.get` reads as a top-level `get`. → `namespaces` + +`other` fails for a fourth reason: `configuration-schema` and the README are +never fetched by anything. + +## Adding or tuning a case + +Append a `Case` to the right list in `cases.py`. + +- `expect` — passes if **any** pattern matches. Case-insensitive. +- `forbid` — fails if **any** pattern matches. Case-**sensitive**, because these + name specific wrong identifiers and `fileName` must not match `filename`. +- `doc_ref` — the exact doc section the answer comes from. +- `why` — the wrong answer you expect. Prints on failure, which is what makes a + red run a worklist instead of just red. +- `target` — keep `"code"` for anything asking for code, so prose *discussing* a + wrong key ("you might reach for `text:`, but…") can't trip a `forbid`. + +Assert the documented fact, not a style preference. A case that fails valid-but- +unfashionable code is a false signal that will mislead whoever reads the score. + +If a probe needs real judgement rather than a string match, it belongs in +`../../acceptance/` as a markdown spec instead. + +## Known limits of this suite + +- The regexes approximate correctness. They have been checked against a right + and a wrong answer each, but a novel phrasing can still fool one — audit + passes as well as failures before trusting a big jump. +- No baseline is recorded here. The numbers measured during development came + from hand-built fixture rows and did not describe the real service, so they + were removed rather than left to be trusted. Establish yours on a machine that + passes the preflight, and record it in your PR. From c244c06cf948c64fc846fd53128d9560b19129e0 Mon Sep 17 00:00:00 2001 From: hanna-paasivirta Date: Thu, 20 Aug 2026 20:00:06 +0100 Subject: [PATCH 5/5] edit readme --- .../job_chat/tests/integration/adaptor_knowledge/README.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/services/job_chat/tests/integration/adaptor_knowledge/README.md b/services/job_chat/tests/integration/adaptor_knowledge/README.md index 469dcdcd..a1de3426 100644 --- a/services/job_chat/tests/integration/adaptor_knowledge/README.md +++ b/services/job_chat/tests/integration/adaptor_knowledge/README.md @@ -76,7 +76,7 @@ retrieval method has to have, and the doc surface it has to reach: | `version` | 5 | You retrieve for the *pinned* version, not the latest | per-version docs | **`signatures` is a smoke test, not a score.** If those three fail, job_chat -isn't getting an adaptor block at all — fix the prerequisites above and rerun. +probably isn't getting an adaptor block at all — fix the prerequisites above and rerun. No other number in that run means anything. **The `version` group catches the obvious shortcut.** Four of its five cases are @@ -140,7 +140,5 @@ If a probe needs real judgement rather than a string match, it belongs in - The regexes approximate correctness. They have been checked against a right and a wrong answer each, but a novel phrasing can still fool one — audit passes as well as failures before trusting a big jump. -- No baseline is recorded here. The numbers measured during development came - from hand-built fixture rows and did not describe the real service, so they - were removed rather than left to be trusted. Establish yours on a machine that +- No baseline is recorded here yet. Establish yours on a machine that passes the preflight, and record it in your PR.