SERPent / CONTRIBUTING.md
Claude
Stop the search chain treating an empty answer as an answer
73c1aa6 unverified
|
Raw
History Blame Contribute Delete
9.7 kB

Contributing to SERPent

This file describes how this codebase is developed, not just how to run it. It exists because the project went from zero tests to a real safety net in one focused pass, and the point of that work is lost if the next change skips the same discipline. It ships as a normal file in this repo, which means it also lands on the Hugging Face Space on every deploy (see "Deployment" below) β€” it isn't GitHub-only documentation.

Setup

python3 -m venv .venv && source .venv/bin/activate
pip install -r requirements-dev.txt
playwright install chromium   # only if not already present on the machine

Dependencies in requirements.txt / requirements-dev.txt are pinned to specific tested ranges, not left bare β€” see the comment at the top of requirements.txt for the policy and how to bump one. This matters more than it looks: coming back to this project after any gap in time, a bare pip install with no pins would silently grab whatever's newest on PyPI that day, and you'd have no way to tell "my change broke this" from "a dependency update broke this."

Running tests

ruff check .   # same lint CI runs
pytest

271 tests, a few seconds, fully offline. All outbound HTTP is mocked with respx; the Playwright-driven scrapers (Bing, Brave, Google Scholar, Google Patents search) run against a real headless Chromium but navigate to local fixture HTML instead of the live sites β€” see tests/helpers.py for how and why. Nothing here depends on, or can trip, any external site's anti-bot measures.

A note on the two backends that are blocked from the deployed Space's IP at the time of writing: Google Scholar serves an anti-bot interstitial rather than results, and query_google_scholar now detects that and raises GoogleScholarBlockedException instead of waiting out a 30s selector timeout. Bing answers normally but wraps every result link in its /ck/a redirector, which decode_bing_redirect unwraps. Both were found by running the live smoke test below against the deployed MCP server, not by the suite - which is exactly the division of labour described next.

Important limitation to keep in mind: this suite protects against regressions in this code. It cannot catch a live site changing its markup, because the fixtures are frozen snapshots of the selector contract as of when they were written β€” they'll stay green forever even if Bing's HTML changes tomorrow. After any real gap in development, or if a scraper starts returning empty/wrong results in production, run a live smoke test against the deployed MCP server or REST API before assuming the code is at fault:

search(["some query"])
scrap_patent("US10000000B2")   # or any real, known-good patent id

If a backend's selectors have drifted, that's a normal, expected kind of bug here β€” fix it the same way as anything else (see below): update the fixture HTML to match the new real markup, confirm the test fails against the current selectors, fix the selectors, confirm green.

How the code is laid out

  • app.py is HTTP wiring only: request validation, dependency providers, the lifespan, and mapping domain errors onto status codes.
  • services.py holds the orchestration policy - which backend to try, in what order, when to fall back, what counts as evidence a backend is unhealthy, and when a patent is genuinely absent rather than merely unreachable. None of that is HTTP-specific, and keeping it out of route handlers is why most of its tests need no ASGI client.
  • serp.py / scrap.py / ops.py are the backends. Each scraper is split into a pure URL builder, a navigation step, and an _extract_* function taking an already-loaded page, so all three can be tested separately.
  • utils.py / circuit_breaker.py are dependency-free helpers.

The services take their stateful collaborators (HTTP client, browser, circuit breaker, OPS credentials) as constructor arguments. That is deliberate and worth preserving: it is what lets a test build a service with its own circuit breaker instead of resetting a process-wide singleton between tests, and hand in a stand-in for the OPS credentials instead of assigning to private attributes on the real token manager. app.py's get_search_service / get_patent_service providers exist so app.dependency_overrides can replace them wholesale in a test.

The stateless backend functions stay module-level imports in services.py; monkeypatching one of those in a test is fine, because they hold no state to leak between tests.

The development loop (TDD, not test-after)

Every change in this codebase's history followed the same loop, and new changes should too:

  1. Write the test first. For new behavior, write it against the behavior you want and confirm it fails (red) for the right reason - not a typo, an import error, or a fixture bug.

  2. For a refactor (behavior should stay identical), write characterization tests for the current behavior first if none exist, confirm they pass against the unchanged code, and only then refactor under that safety net. app.py had zero tests before its endpoint deduplication; the tests were added and confirmed green against the old code before a single line of the refactor happened.

  3. Implement the minimal change to make the new/updated test pass.

  4. Run the full suite, not just the new test - a shared helper's bug should show up as multiple failures, and that's a sign it's finding real things, not a nuisance.

  5. Distrust a test that never fails. When a test's value is in question, deliberately break the code it's supposed to protect and confirm the test actually catches it, then revert. This caught a real gap once already: a circuit breaker test that looked correct was actually vacuous until this step exposed it.

    Applying this systematically is worth a pass of its own, and one such pass is where most of the tests added since came from. Line coverage was 93% and hid the problem completely: the suite killed five of six mutants aimed at pure functions, and one of seven aimed at the I/O boundary. Deleting the entire API-key middleware left it green, as did replacing the Bing search URL with a constant. The lesson worth keeping is where to point this technique - at the code that touches the outside world, not the code that's easy to test.

    A useful heuristic fell out of it: if a property has no behavioural signature at all, no black-box test can pin it. == and secrets.compare_digest return the same answer for every input and differ only in timing, so that one test asserts structurally that the comparison goes through compare_digest. Reach for that only when there is genuinely nothing observable to assert on.

  6. Watch for shared mutable state across tests. A few characterization tests in this codebase turned out to be recording real state into a module-level singleton (a circuit breaker) with nothing resetting it between tests - harmless by luck at the time, but the kind of thing that silently breaks an unrelated, later test once one more test is added. Give shared singletons a fresh instance per test (an autouse fixture, or explicit monkeypatching) rather than relying on the real one.

  7. Validate risky changes for real, not just by review. The one Docker change in this project's history that could plausibly have broken the live deployment (running the container as non-root, which changes whether Chromium's sandbox can start at all) was verified by actually building and running the image locally and confirming a real scrape request completed - not by reasoning about it from the Dockerfile alone.

CI / deployment

  • .github/workflows/test.yml runs ruff check . and then the full suite on every pull request against main, so a change gets feedback before merge. The lint rule set (see pyproject.toml) is deliberately small - pyflakes-equivalent checks plus bugbear - because the first pass over this repo found a duplicate import shadowing another, four unused imports and an f-string with no placeholders. Widen it when the team wants more rather than starting broad and accumulating ignores.
  • .github/workflows/deploy-to-hf.yml runs the same suite again (test job) on every push to main, and only then (needs: test) pushes the repo to the Hugging Face Space (deploy job) with git push --force-with-lease. That flag matters: a plain --force push is a blind overwrite with no way to tell if someone else pushed to the same Space in the meantime; --force-with-lease fails loudly instead of silently clobbering a concurrent change. If you ever see that step fail with "stale info," it means exactly that - fetch, look at what's there, and reconcile before retrying.
  • Because the deploy step pushes the whole repo tree (HEAD:main), not a curated subset, everything here - including this file, the tests, and the CI config - ends up on the Space too. There's nothing GitHub-only about this process.

Secrets to know about

These expire or rotate silently and are the most likely source of a deploy failure that has nothing to do with the code:

  • HF_TOKEN (repo secret) - used to push to the Hugging Face Space.
  • OPS_CONSUMER_KEY / OPS_CONSUMER_SECRET (optional, set on the Space) - EPO OPS API credentials for the patent-search fallback.
  • SERPENT_API_KEY (optional, set on the Space) - if set, gates every request behind an API key; if you set this and then can't reach the deployed API, check here first.