Skip to content

feat(scripts): model what each example under examples/ requires - #597

Open
williedoran-neo4j wants to merge 4 commits into
neo4j:mainfrom
williedoran-neo4j:willie/examples-requirement-model
Open

feat(scripts): model what each example under examples/ requires#597
williedoran-neo4j wants to merge 4 commits into
neo4j:mainfrom
williedoran-neo4j:willie/examples-requirement-model

Conversation

@williedoran-neo4j

@williedoran-neo4j williedoran-neo4j commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

First of four PRs. The order matches how the tools get used: work out what each example needs (this) → report what is missing (#599) → walk through fixing it (#602) → run what the machine supports (#598).

Nothing knows what an example needs in order to run, so a contributor finds out by running it and reading the traceback. This adds one source of truth the rest of the tooling shares.

Where requirements come from

  • Packages, from an AST import scan that maps library symbols to extras. An example imports OpenAILLM, not openai, so the dependency is invisible from its imports alone. The substring scan this replaces treated any file containing the word "google" as a Vertex example and could not tell OpenAIEmbeddings from AzureOpenAIEmbeddings.
  • Env vars and datastores, read from os.getenv calls and connection URIs in the source.
  • Everything else — APOC, a pre-existing index, outbound internet, or a component named only in a YAML config — declared per path in SERVICE_RULES, because it leaves no trace in the source.

Stdlib-only by design: it has to run before uv sync, which is exactly when somebody most needs to be told what to install.

Keeping the tables honest

The tables are hand-maintained, and a half-finished edit produced a plausible wrong answer rather than an error. Each of these is a one-line test:

  • SYMBOL_EXTRAS and SYMBOL_PROVIDERS are two halves of one fact. Their values legitimately differ — AzureOpenAILLM needs the openai extra but talks to the azure provider — so they cannot be merged, but their keys must not drift. A symbol in one and not the other means the doctor installs a package it never asks for a key for, or the reverse.
  • Every service an example needs has a label. service_available() returns True for anything it cannot probe, so a typo'd service name would otherwise report as satisfied.
  • SERVICE_RULES matches are pinned by count. Asserting only that a pattern matches something protects the nine rules matching exactly one file; the other ten match several, so renaming one of them leaves the pattern non-empty and the requirement silently lost. The count also catches a glob widened until it sweeps in a file that does not want the requirement. Update it deliberately when examples/ gains or loses a file — that is the moment to confirm the rule still applies.

SERVICE_RULES specs are a TypedDict rather than dict[str, object]. Read back through spec.get(), a mistyped key ("service") or a scalar where a list belongs both silently did nothing. mypy already runs over scripts/ in CI and now rejects both.

Two CI commits, which do not belong to the model

Called out because they are unrelated to everything above, and would otherwise be a surprise in the diff.

Four e2e jobs failed on #596 with ImportError: cannot import name 'WeaviateNeo4jRetriever' — and the same for Pinecone and Qdrant — on a commit that changed two Markdown files. No test was flaky: the job restored a cached .venv missing the vector-store extras and, because installation was skipped on an exact cache hit, nothing repaired it. Re-running reproduced it exactly; deleting the cache entry fixed it.

  • ci: always install dependencies, even on a cache hit stops a bad entry causing failures.
  • ci: never publish a half-installed venv to the cache stops one being created: actions/cache saves in a post step that also runs when a job is cancelled, so an interrupted uv sync publishes the partial venv. Splitting into restore/save makes the save an ordinary step, which an interrupted install never reaches.

They are here rather than in their own PR to avoid opening another one; happy to lift them out if a maintainer would rather they merged independently, since they fix CI for everyone and this stack is four deep.

Scope

Deliberately just the model, its tests and those CI fixes. The model has no consumer until #599 — which is what keeps this small enough to check by reading.

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Documentation update
  • Project configuration change

Complexity

Complexity: Low

How Has This Been Tested?

  • Unit tests
  • E2E tests
  • Manual tests

29 tests over the scan, the per-path rules and the table invariants. Each new invariant was confirmed to fail when the thing it guards is broken — dropping a symbol from one table, and simulating a rename against the rule counts — rather than merely passing today.

scripts/ sits outside the coverage gate ([tool.coverage.run] source = ["src"]), so these tests are additive and cannot move the 90% number. They need one line of wiring, pythonpath = ["scripts"].

Checklist

  • Documentation has been updated
  • Unit tests have been updated
  • E2E tests have been updated
  • Examples have been updated
  • New files have copyright header
  • CLA (https://neo4j.com/developer/cla/) has been signed
  • CHANGELOG.md updated if appropriate

🤖 Generated with Claude Code

@williedoran-neo4j
williedoran-neo4j force-pushed the willie/examples-requirement-model branch from f956372 to b1bab9e Compare August 11, 2026 10:16
@williedoran-neo4j
williedoran-neo4j force-pushed the willie/examples-requirement-model branch 3 times, most recently from 31ef2cc to e3a12f3 Compare August 12, 2026 10:30
@williedoran-neo4j williedoran-neo4j changed the title feat(scripts): model what each example requires, and use it in the checker feat(scripts): model what each example under examples/ requires Aug 12, 2026
@williedoran-neo4j
williedoran-neo4j force-pushed the willie/examples-requirement-model branch from e3a12f3 to 29fafdb Compare August 12, 2026 12:04
williedoran-neo4j and others added 2 commits August 13, 2026 10:35
Nothing knows what an example needs in order to run, so a contributor finds out
by running it and reading the traceback. This adds one source of truth the
tooling can share.

Requirements come from three places. Packages are resolved by an AST import scan
that maps *library symbols* to extras - an example imports OpenAILLM, not openai,
so the dependency is invisible from its imports alone. Env vars and datastores
are read from os.getenv calls and connection URIs in the source. Anything that
leaves no trace - APOC, a pre-existing index, outbound internet, or a component
named only in a YAML config - is declared per path in SERVICE_RULES.

Stdlib-only by design: it has to run before uv sync, which is exactly when
somebody most needs to be told what to install.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All three workflows skipped `uv sync` whenever the venv cache key hit exactly.
That trusts the cache to be complete, and it is not always: a cache saved from
an interrupted install is restored on every later run with the same key, and the
install that would repair it is precisely what gets skipped.

Seen on neo4j#596, which changes uv.lock and so minted a fresh key: four e2e jobs
failed with `ImportError: cannot import name 'WeaviateNeo4jRetriever'` and the
same for Pinecone and Qdrant. The restored .venv was missing the vector-store
extras, the test step installed only the 82 default packages, and re-running
reproduced it exactly - the same cache was restored each time. Deleting the
cache entry fixed it.

The failure mode is nasty out of proportion to its cause: it presents as a
flaky test in code that did not change, it survives re-runs, and it persists
until the cache expires or somebody deletes it by hand. Any PR touching uv.lock
is first to hit a new key, so it lands on whoever is least likely to suspect CI.

uv sync takes a few seconds when the environment is already correct, which is
cheap next to that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@williedoran-neo4j williedoran-neo4j changed the title feat(scripts): model what each example under examples/ requires feat(scripts): determine what model provider each example under examples/ requires Aug 13, 2026
@williedoran-neo4j
williedoran-neo4j force-pushed the willie/examples-requirement-model branch from af0a41d to d9b6101 Compare August 13, 2026 08:37
williedoran-neo4j and others added 2 commits August 13, 2026 11:05
Review feedback on neo4j#597. Two of the tests did not exercise what they claimed,
and nothing guarded the tables against a partial edit.

- test_an_aliased_symbol_still_maps_to_its_extra never fed the parsed symbols to
  anything: it built an ExampleRequirements by hand and asserted its extras were
  empty, which is true by default_factory whatever the code does. The symbol to
  extra join it names was never executed.
- test_install_hints_reports_a_package_behind_no_extra asserted the corpus
  reports nothing, which is also what the bug it names produced.

Both reached for hand-built objects because analyse() raised ValueError on any
path outside examples/, so a synthetic file could not be analysed at all. It now
returns instead, and both tests run the real pipeline over a temp file.

SERVICE_RULES specs are now a TypedDict. They were dict[str, object] and read
back through spec.get(), so a mistyped key ("service") or a scalar where a list
belongs both silently did nothing - the example quietly lost its requirement.
mypy already runs over scripts/ in CI and now rejects both.

Adds the invariants that were missing entirely. SYMBOL_EXTRAS and
SYMBOL_PROVIDERS are two halves of one fact: their values legitimately differ,
since AzureOpenAILLM needs the openai extra but talks to the azure provider, but
a symbol in one and not the other means the doctor installs a package it never
asks for a key for, or the reverse. Nothing caught that. Nor a service name no
label covers, which service_available() reports as satisfied because it cannot
probe it.

SERVICE_RULES matches are now pinned by count. Asserting only that a pattern
matches something protects the nine rules matching exactly one file; the other
ten match several, so renaming one of them leaves the pattern non-empty and the
requirement silently lost. The count also catches a glob widened until it sweeps
in a file that does not want the requirement.

Also: read_text() now specifies utf-8, one example being non-ASCII; a duplicate
sibling_modules assignment is removed; and install_hints' test is renamed to what
it actually asserts, the import name rather than the distribution.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the previous commit. Making the install unconditional stops a bad
cache entry causing failures, but does not stop one being created.

actions/cache saves in a post step, which also runs when the job was cancelled
or failed - so a run interrupted during uv sync publishes a half-installed .venv
under a key derived from uv.lock alone. Splitting into restore/save makes the
save an ordinary step, so an interrupted install never reaches it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@williedoran-neo4j williedoran-neo4j changed the title feat(scripts): determine what model provider each example under examples/ requires feat(scripts): model what each example under examples/ requires Aug 13, 2026
@williedoran-neo4j
williedoran-neo4j marked this pull request as ready for review August 13, 2026 09:13
@williedoran-neo4j
williedoran-neo4j requested a review from a team as a code owner August 13, 2026 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant