Skip to content

CI: run the gated notes LLM e2e per-PR as a blocking check (path-scoped, hosted Ollama) #589

Description

@goosewobbler

Follow-up to the LLM-notes reliability work (#585), which added a gated real-provider e2e at packages/notes/test/integration/notes-llm-e2e.spec.ts (opt-in via RELEASEKIT_NOTES_E2E=1, skipped by default). Today it only runs when invoked manually, so the real-provider path (chunking across the >30-entry boundary, structured output, the free-text summarize/release-notes path) is never exercised in CI and can rot.

Decision

Run it per-PR as a blocking check, made safe by two constraints. The maintainer has an Ollama Cloud subscription and this uses a cheap model, so the earlier cost/speed objection to per-PR runs no longer applies; per-PR real-provider verification is worth having.

Two things keep "blocking" from being fragile:

  1. Path-scope to notes changes. Gate the job behind the existing detect-changes / paths-filter so it only runs on PRs touching packages/notes/** (and this spec / shared LLM code). Unrelated PRs never depend on Ollama, shrinking the "external service blocks merges" surface to notes PRs only, and avoiding needless API calls.
  2. Infra-tolerant test. The spec must fail on wrong output (a real pipeline regression) and skip — not fail — on an unreachable provider (connection error / timeout / 5xx / 429). A malformed-request 4xx still fails (that's our regression, not infra). This keeps a red check meaningful without turning ollama.com's uptime into a merge gate. Reuse the retry classifier's infra-vs-logic distinction (isRetryableProviderError).

Scope

Prerequisites / notes

  • Add the OLLAMA_API_KEY repo secret (maintainer action).
  • Sequence after ci: harden workflows — SHA-pin actions, drop inlined SSH key, least-privilege #583 (CI workflow hardening) merges to avoid workflow conflicts.
  • The spec's assertions are already CI-safe (structure-only; the per-chunk fallback preserves entry counts even when the model mangles a structured response), so with infra-tolerance the only way it fails is a genuine pipeline regression.

Related: #585 (added the test), #542 (eval harness — the deterministic, replay-based per-PR coverage; this real-provider check complements it).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:ciUpdates to Continuous Integrationarea:testsUpdates to teststype:enhancementNew feature or request

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions