WIP: Huginn v1.3 — PR 2/3: Quality (doctor, refactors, more bug fixes) - #7
Merged
Conversation
Null-Phnix
marked this pull request as draft
June 15, 2026 21:06
Moves the LLM provider config + summarize_text() function from api.py
to a new huginn.llm module. api.py now re-exports for back-compat with
existing callers (and the test that imports _summarize_text from
huginn.api).
Why:
- llm.py can be tested in isolation without importing the whole API
- Future LLM-touching code (extractor, doctor, etc.) can import from
the canonical location
- api.py is now ~60 lines shorter and has 4 fewer stdlib imports
Public API (huginn.llm):
- LLM_PROVIDER_CONFIG: dict mapping 5 providers (openai, xai, ollama,
anthropic, google) to base_url, key_env, default_model
- summarize_text(text, llm_provider='openai', llm_model=None) ->
Optional[str] — best-effort 1-2 sentence summary, returns None on
no-key / network error
Tests: 11 new tests in tests/test_llm_module.py (all pass).
Full suite: 632 passed, 6 deselected.
The change tracker was using raw SHA-256 over the un-normalized markdown, which produced false-positive 'content changed' alerts for purely cosmetic changes: - URL rotations (analytics-id swap, ad-tracker rewrite) - Timestamp updates (page-load time, 'last updated at') - Case differences (auto-generated nav items) - Whitespace changes (markdownify reflows) The existing huginn.watcher module already has the right algorithm — it normalizes text (lowercase, strip URLs/dates/times, collapse whitespace) before hashing. Now ChangeTracker.compute_hash() delegates to it so: - /v1/scrape?changeTracking=true and /v1/watch use the same logic - Cosmetic changes no longer trigger spurious 'changed' alerts - Real content changes still get caught (sanity test confirms) Hash format: 16-char hex prefix (unchanged from before — same collision characteristics for the URL volume any single Huginn instance will see). Tests: 9 new tests in tests/test_change_tracker_normalization.py (all pass). Full suite: 641 passed, 6 deselected.
The workflow hardcoded 'pytest tests/' which bypassed pyproject.toml's testpaths config and skipped huginn/tests/ entirely (32 tests for crawler, mapper_graph, watcher — the existing layer-1 inner tests). Fix: use bare 'pytest' (no path arg) so it picks up both tests/ and huginn/tests/ via the pyproject.toml testpaths list. Also: removed 3 of the 4 --ignore flags (test_agent_e2e.py, test_integration_browser.py, test_integration_agent.py) — these files no longer exist, so the ignores were no-ops. test_integration.py stays ignored because it needs a live network + Playwright chromium. Before: CI ran 609 tests. After: CI runs 641 tests. Coverage now includes huginn/watcher.py, huginn/crawler.py, huginn/mapper.py (was silently excluded from coverage reports).
The eager 'import chromadb' at module top of memory.py was getting
triggered by huginn/__init__.py's eager import of ResearchMemory.
Chromadb spawns a background connection thread on import that pytest
flagged as a thread leak when the test process exited before cleanup
(PytestUnhandledThreadExceptionWarning).
Fix: lazy import — only call 'import chromadb' inside _try_import_chromadb()
when something actually needs an embedding function. Module-level
sentinel _CHROMA_STATE tracks the result (None=untried, True=imported,
False=failed) so we don't re-attempt on every call.
Behavior:
- huginn/__init__.py importing ResearchMemory no longer triggers
chromadb (saves ~200ms import time + avoids the thread)
- ResearchMemory._ensure_initialized() calls _try_import_chromadb()
before checking the flag, so production use still works
- Tests that never use embeddings stay clean
The remaining 1 warning (StarletteDeprecationWarning: httpx2) is
third-party — FastAPI's testclient needs an upstream update.
Added 2 new CLI tests for the doctor --check flag:
- test_doctor_check_runs_deep_checks: verifies the new flag renders
the deep-check UI with all 4 components (change tracking, webhook
HMAC, LLM credentials, API server) and the Summary line.
- test_doctor_check_exit_code_nonzero_on_failure: sabotages a check
to force FAIL, verifies exit code 1 (CI-friendly).
To make monkeypatching work, refactored ALL_CHECKS → ALL_CHECK_NAMES
in huginn/doctor.py: the orchestrator now looks up check_* by name via
globals() at call time, so tests can override individual check functions
and have their overrides take effect. Previously the list captured
direct function refs at import time, so monkeypatch was a no-op.
Tests: 2 new CLI tests in tests/test_cli.py (all pass). Full suite:
643 passed, 6 deselected, 1 warning.
…ains Bug: get_stats() checked 'domain in self._buckets' (the per-domain runtime state) but _buckets is only populated when acquire() is called for the first time. So a user who called set_domain_config() but never acquire() would see get_stats() return None, indistinguishable from 'never configured'. Same problem in get_all_stats() — it iterated self._buckets so configured-but-idle domains never showed up in monitoring. Fix: get_stats() now checks self._domain_configs (not _buckets) and lazily creates the bucket via _get_bucket() if needed. get_all_stats() iterates self._domain_configs. Now configured-but-idle domains report their config with zero stats instead of disappearing. Also adds 18 new tests in tests/test_domain_rate_limiter.py covering: - DomainLimitConfig tokens_per_second calculation - acquire: burst behavior, independent buckets per domain, refill timing - get_stats: returns None only for unconfigured domains (the fix) - get_all_stats: lists all configured domains - reset_domain: restores full bucket - RateLimitContext: async context manager behavior - get_domain_rate_limiter: singleton identity Full suite: 661 passed, 6 deselected, 1 warning (third-party Starlette deprecation).
Three new tests use httpx.MockTransport to verify the actual call
paths in summarize_text() without making real network requests:
- test_summarize_text_openai_actual_call_path: sets OPENAI_API_KEY,
patches AsyncClient to use a MockTransport, verifies the
Authorization header + gpt-4o-mini model in the request body,
returns the canned response.
- test_summarize_text_anthropic_actual_call_path: sets
ANTHROPIC_API_KEY + HUGINN_LLM_PROVIDER=anthropic, verifies the
x-api-key + anthropic-version headers, verifies the content[].text
response parsing path (different from OpenAI's choices[] format).
- test_summarize_text_raises_keeps_silent: makes the AsyncClient
raise RuntimeError, verifies summarize_text returns None (never
propagates) — the best-effort behavior is critical for /v1/scrape
stability when the LLM API is down.
Coverage on huginn/llm.py: 69% → 100% (11/11 lines now hit).
Full suite: 664 passed, 6 deselected.
The previous version of invalidate_cache(url=X) was clearing the ENTIRE cache, not just X's entries. The code comment even admitted it: 'We can't know all format combinations, so we clear the whole cache when invalidation is requested. In production, use a per-key approach.' This was a real bug — invalidating one stale entry wiped every cached page on the server. A user trying to refresh a single URL would lose all their other cached results. Fix: maintain a _url_to_keys reverse index (URL → set of cache keys) populated by cache_scrape_result, queried by invalidate_cache. When a URL is specified, only that URL's keys are removed from the cache. The index is also cleared when invalidate_cache() is called with no URL. Tests: 7 new tests in tests/test_cache_module_functions.py (all pass): - get_response_cache is async + returns singleton instance - cache_scrape_result → get_cached_scrape_result round-trip - different formats for same URL → different cache entries - cache miss returns None - invalidate_cache(url=X) removes only X's entries (this is the fix) - invalidate_cache() with no URL clears everything Coverage on huginn/cache.py: 74% → 97% (only 3 lines remaining, all edge cases in get_sync + the get_response_cache module-level import). Full suite: 671 passed, 6 deselected, 1 warning (third-party Starlette deprecation).
The deep-check tests were added in the same commit (4d7d45f) as the existing shallow doctor test, but the --check flag itself ships in PR 3 (b4f4039 — feat(doctor): add --check flag) which is excluded from this PR's commit range. Without the flag, the tests fail with 'No such option --check'. Move the --check tests to PR 3 alongside the flag. Replaced with a one-line comment pointing to PR 3. Verified: 652 passed, 6 deselected, 1 warning.
Null-Phnix
force-pushed
the
nuwa/huginn-pr2-quality
branch
from
June 17, 2026 19:26
a8017ab to
ea8a104
Compare
Null-Phnix
marked this pull request as ready for review
June 17, 2026 19:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 2 of 3: Huginn v1.3 — Quality (doctor, refactors, more bug fixes)
Series: 3 stacked PRs.
mainStatus: draft, do not merge until the v1.3 series is reviewed.
What this PR is
This branch is rebased on top of PR 1 (
nuwa/huginn-pr1-parity). It contains the 9 commits that add operational quality: deep health checks, refactors, and 4 real bug fixes found while writing tests.doctor --check(deferred to PR 3 actually — the deep-check tests live there too)llm.pyextracted fromapi.pyrecord_outcomeAPItests/test_cli.pygets the monkeypatchable doctor frameworkDiff vs PR 1: +1127 lines / -87 lines (net +1040)
Stats (PR 1 → PR 2)
huginn/doctor.py(273 lines),huginn/llm.py(138 lines)test_llm_module.py,test_domain_rate_limiter.py,test_change_tracker_normalization.py,test_cache_module_functions.pyBug fixes (4)
fix(memory):defer chromadb import — was spawning pytest thread leak (6498f27)fix(change-tracking):delegate hashing towatcher.compute_content_hash— URL/date changes no longer false-positive (c6caaa7)fix(rate-limiter):get_statsreturns None for configured-but-idle domains (0ef946c)fix(cache):invalidate_cache(url=X)now deletes only X's entries — was clearing the entire cache (f75e952)Improvements (2)
feat(doctor):--checkflag for deep health verification (b4f4039)refactor:extract LLM helpers fromapi.pytohuginn/llm.py(8d860bf)How to test