CONTRIBUTING.md

# 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

```bash
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

```bash
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.