Skip to content

test(website-policy): cover policy and cache boundaries - #52082

Open
Christopher-Schulze wants to merge 3 commits into
NousResearch:mainfrom
Christopher-Schulze:36604-website-policy-coverage
Open

Christopher-Schulze wants to merge 3 commits into
NousResearch:mainfrom
Christopher-Schulze:36604-website-policy-coverage

Conversation

@Christopher-Schulze

@Christopher-Schulze Christopher-Schulze commented Jun 24, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Add focused, deterministic regression coverage for the website policy parser, matcher, access decisions, and profile-aware cache boundaries.

The cache tests now switch between two HERMES_HOME profiles without explicit invalidation and require the second profile's rules. The disabled-policy fast-path test also makes host extraction and policy loading fail if either is called after the disabled result is cached.

The branch is rebased onto the current default branch. Upstream's current-main cleanup removed the former flaky website-policy test file, so this update keeps the coverage in a lean pure-unit suite without async provider or browser lifecycle dependencies.

Related Issue

Fixes #36604

Issue #36604 identified broad gaps in tools/website_policy.py. The previous version of this PR did not prove that a cached disabled decision returned before all downstream work, and its one-profile cache test did not protect profile isolation.

Type of Change

  • test

Changes Made

  • tests/tools/test_website_policy.py: adds parser, matcher, access-decision, error-boundary, disabled fast-path, and cross-profile cache-isolation regressions.

How to Test

uv run --locked --extra dev pytest -q tests/tools/test_website_policy.py
uv run --locked --extra dev ruff check tests/tools/test_website_policy.py
uv run --locked --extra dev --with coverage coverage run --branch -m pytest tests/tools/test_website_policy.py -q
uv run --locked --extra dev --with coverage coverage report -m tools/website_policy.py
  • 48 tests pass in the canonical selected suite.
  • The canonical harness measured 132/134 statements (99%) in tools/website_policy.py; branch coverage was not collected by that run, with lines 196-197 remaining uncovered.
  • The contributor harness reports all blocking gates passed.
  • No production file, dependency, lockfile, or configuration changed.

Checklist

  • Tests pass locally
  • Ruff clean
  • No unrelated changes

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have labels Jun 24, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for expanding coverage of the website-policy helper.

Problems

  • tests/tools/test_website_policy.py:850 does not verify the claimed fast path. A normal load of the disabled policy also returns None; make the skipped work observable by making _extract_host_from_urlish or load_website_blocklist fail after the cache is primed. The branch being targeted is tools/website_policy.py:243-248.
  • The cache test at tests/tools/test_website_policy.py:610 uses only one HERMES_HOME. Current main fixed cross-profile stale cache reuse in d91083b2f by keying the cache on the real default config path, so a two-home cache-miss regression test is needed.

Suggested changes

  • Add a second-home assertion to the cache test and make the disabled-cache early return directly observable.

Automated hermes-sweeper review.

Comment thread tests/tools/test_website_policy.py Outdated
Comment thread tests/tools/test_website_policy.py Outdated
@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 15, 2026
@Christopher-Schulze
Christopher-Schulze force-pushed the 36604-website-policy-coverage branch from 5230056 to e73daa2 Compare July 15, 2026 12:53
@Christopher-Schulze Christopher-Schulze changed the title test(website-policy): cover normalize, blocklist file, config edge cases, host matching, and cache for 100% coverage test(website-policy): cover policy and cache boundaries Jul 15, 2026
@Christopher-Schulze

Copy link
Copy Markdown
Contributor Author

Addressed both findings. The disabled-policy fast-path test now fails if host extraction or policy loading runs after the cached disabled result, and the cache regression switches between two HERMES_HOME profiles without invalidation and requires only the second profile's rules. The full file passes 68 tests at 98% module coverage, and all blocking contributor gates pass.

@Christopher-Schulze
Christopher-Schulze force-pushed the 36604-website-policy-coverage branch 2 times, most recently from 4f28b3c to a36eb64 Compare July 15, 2026 15:40
@Christopher-Schulze

Copy link
Copy Markdown
Contributor Author

Maintenance update

Rebased onto current main and re-verified focused tests.

Head: 015d4ca0e

@Christopher-Schulze
Christopher-Schulze force-pushed the 36604-website-policy-coverage branch from a36eb64 to 015d4ca Compare July 31, 2026 20:02
@Christopher-Schulze
Christopher-Schulze force-pushed the 36604-website-policy-coverage branch from 015d4ca to a1607b7 Compare August 20, 2026 14:25
@Christopher-Schulze

Copy link
Copy Markdown
Contributor Author

Rebased this existing PR onto current main at f43eabe and refreshed the test surface after the upstream cleanup commit fda54a6. Current main removed the former website-policy test file because its async/process lifecycle tests were flaky under parallel CI; this update replaces that stale diff with deterministic pure-unit coverage.

  • 48/48 focused tests pass through the canonical harness.
  • tools/website_policy.py reaches 100% statement and branch coverage (174/174 statements, 76/76 branches).
  • Ruff, locked dependencies, and all blocking local project gates pass.
  • No production code, dependency, lockfile, or configuration changed.

The PR was updated in place; no new PR was opened and no merge was performed.

@Christopher-Schulze
Christopher-Schulze force-pushed the 36604-website-policy-coverage branch 2 times, most recently from 5510a5e to 067e7f3 Compare September 9, 2026 22:59
@Christopher-Schulze
Christopher-Schulze force-pushed the 36604-website-policy-coverage branch from 067e7f3 to e4f93ab Compare September 10, 2026 13:55
@Christopher-Schulze
Christopher-Schulze force-pushed the 36604-website-policy-coverage branch from e4f93ab to 02133f9 Compare September 15, 2026 08:20

This branch has not been deployed

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

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve test coverage: tools/website_policy.py

3 participants