Simplification: all 9 phases + audit fix + slow-suite restoration (plan 1 of #211) - #215
Merged
Conversation
The 5-class hierarchy (HTMLEscapeConstants, HTMLEscaper, RelayFieldEscaper, BulkRelayEscaper + factory) is now two module-level functions: escape_relay_fields(relay) (the single bulk per-relay pass production uses, field set unchanged) and safe_html_escape, plus the four fallback constants. 411 -> 101 lines (before dead-code batches already removed part of the old module). Byte gate: noise floor only; pytest 1012. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The handle_http_errors decorator's 304/HTTP/network handling moved into workers._fetch_with_cache_fallback, where api_name/display_name/ log_progress/cache helpers already exist; the decorator (and its fragile positional-args logger inspection) is deleted. Per-API semantics live on APIConfig as allow_exit_on_304/critical fields (details True/True, others False/False - matching the old decorator invocations exactly). Message strings preserved byte-for-byte; the unreachable inner 'except HTTPError: raise' clause removed. Behavior deltas (both intentional, documented): - bandwidth error messages now route through the passed progress_logger instead of raw print (the old args[1] inspection could never find bandwidth's logger because cache_hours occupies position 1) Error-path parity proof: 55-scenario harness (5 fetchers x 11 error/ cache scenarios) capturing ordered log stream + results + worker status, byte-identical before/after except the documented logger routing. Tests 3 -> 8 in test_http_error_cache_keys.py, now mocking only the network primitive so they survived the refactor unchanged. Full-site gate: 17 diffs all clock noise; pytest 1017. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
One ProgressLogger instance now threads allium.py -> coordinator -> Relays -> page_writer/site_generator; deleted: coordinator's _log_progress wrapper trio and mirrored step attributes, Relays._log_ progress and its start_time/progress_step/total_steps constructor params, the dead self.progress_step increments the coordinator used to overwrite, progress_logger.py's unused log_with_increment/get_current_ step/set_step/increment_step/create_progress_logger and the legacy stub. No step ints cross module boundaries anymore. +93/-187. Progress format proof: details-mode smoke runs (pinned API) before vs after are byte-identical after stripping timestamps/RSS, ending at the derived [49/49]; full run still [61/61] with identical step sequence. ProgressLogger pickle round-trip verified (fork pool). Full-site gate: 14 diffs all clock noise; pytest 1017. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- bandwidth_utils.process_all_bandwidth_data_consolidated's inline mean/stdev/two-sigma block delegates to StatisticalUtils.calculate_basic_statistics (uptime_utils already delegated everything; the CV percentile code was deleted in LOC B1) - relay_diagnostics._format_rate and consensus_evaluation._format_ bandwidth_value shared-formatter caching consolidated into bandwidth_formatter._get_formatter; private cache dicts deleted - overload detection merged into stability_utils.evaluate_overload (single source for the three onionoo overload fields, ms->hours math, 72h window); the callers' documented semantic differences (stability treats any truthy fd_exhausted as overloaded and reports stale overloads unboundedly; diagnostics requires dict fd data and caps at 7 days) are parameterized, not normalized - api_diagnostics._format_timestamp delegates to time_utils.format_timestamp_gmt (gained optional fmt param); _format_age/_format_time_ago left: duration formatters with no time_utils counterpart - the three byte-identical url: token regexes alias one string_utils.URL_FIELD_TOKEN_RE (aroi_validation keeps the _URL_FIELD_RE name so relays.py's hot-path import is untouched) Proof: 2,352 old-vs-new fixture comparisons at threshold boundaries, all identical (old = git show from branch head); one crash-only divergence documented (truthy non-dict fd_exhausted no longer raises). Full-site gate: all diffs clock noise; pytest 1017. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
coordinator.API_WORKER_REGISTRY (parallel dict list) is gone; APIConfig gains worker_fn/args_fn/group/progress_name/enabled/feature_flag and workers.API_CONFIGS is the single registry the coordinator derives workers, gating, and progress display names from. The two CollecTor workers get real APIConfig entries (feature_flag='consensus_evaluation' replaces the ad-hoc enabled_fn), and the planned consensus-health path is registered but dormant (enabled=False) instead of remaining an orphan fetcher - kept per plan decision. Coordinator's hardcoded _get_api_display_name if/elif chain now reads config.progress_name. Byte gate: noise floor; pytest 1017; progress steps still [61/61] (derivation reads the same iterator). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The five misc-*.html listings shared ~60% of their markup (BW/CW/ relay-count sort-header matrices, network-relays bullet, last-updated paragraph, row data cells) byte-identically modulo the URL prefix. Extracted into misc_listing_macros.html (bw_headers/cw_headers/ rc_headers/sortable_th/listing_row_cells/...), 876 -> 428 lines across the five pages + 141-line macro file (net -307). misc-families keeps its row cells local (different indentation and an interleaved Contact cell - not byte-identical, not extracted). Byte-identity proof: 170 full-page renders (5 templates x 17 sort variants x base_url set/unset, rows covering all validation states, v3 tiers, missing fields) rendered == before/after against this head. Full-site gate: noise floor; pytest 1017. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The validated-AROI-domain vs contact-page URL conditional was duplicated in four places (detail_summary in macros.html; operator_link, champion_badge, top3_table in aroi_macros.html). New operator_href macro in macros.html is the single source; context-free import semantics (base_url undefined fallback) preserved. Families B and C from the plan are REFUTED for byte-identical extraction and left unchanged: relay-list.html is tab-indented vs contact-relay-list.html space-indented inside literal output lines (one macro cannot emit both byte sequences), and country.html's summary bullets differ from detail_summary in user-visible tooltip text - merging would silently normalize content. Proof: 40 probe renders == before/after; gate: noise floor; pytest 1017. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fluent builder was internal-only (StandardTemplateContexts was its sole consumer; zero external/test references). Its add_* methods are now direct dict construction inside the four StandardTemplateContexts methods, preserving exact update order. page_context.py 330 -> 235 lines. (The file_io_utils deletions from this plan phase landed in LOC Batch 3; page_writer's ignored relay_set param is handled with the facade removal in Phase 5.) Byte gate: noise floor; pytest 1017. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removed write_misc, write_pages_by_key, write_relay_info, _build_template_args, and the dead get_detail_page_context/ _write_pages_parallel delegates; call sites in site_generator, the page_writer mp workers (module functions pickle by name), and tests now call the page_writer module functions with relay_set explicit. Batch proof (agent worktree, frozen-time smoke vs pristine base): entire generated tree byte-identical, 0 diffs; pytest 1017. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removed _generate_contact_rankings, _calculate_operator_reliability, _compute_contact_display_data, _get_contact_validation_status (its getattr-default logic preserved in page_writer._contact_validation_ status, used by both forked precompute paths) and 5 zero-caller delegates. 32 test call sites migrated to module functions. Batch proof: frozen-time smoke byte-identical incl. every contact page; pytest 1017. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removed _format_time_ago, _calculate_uptime_display (hot-loop sites call module functions directly), _calculate_network_health_metrics, _format_intelligence_rating, and the dead _get_leaderboard_category_ info/_preformat_network_health_template_strings. KEPT with reasons: _get_directory_authorities_data (page_context caller; import cycle), _aroi_validation_timestamp (cached_property with real logic), _generate_aroi_leaderboards (state bump + patched in tests), _generate_smart_context (real orchestration, no module twin). Batch proof: frozen-time smoke byte-identical; pytest 1017. Also surfaced: search-index.json as_names ordering is nondeterministic on UNMODIFIED code (thread as_completed merge) - pre-existing, flagged separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n coordinator Step 2: write_pages_by_key's sequential loop dropped its ~120-line inline arg construction and now calls the same build_template_args as the parallel workers - the two paths can no longer drift. (Step 1's unreachable contact branches were removed in LOC Batch 2.) Step 3: AuthorityMonitor TCP probing moved out of page rendering into the coordinator (new page_writer.probe_authority_latency, attached as relay_set.authority_latency_status after the relay set is built); get_directory_authorities_data consumes the attached result and only probes inline as a fallback for tests/direct calls. Page generation is now pure rendering. Proof (frozen-time full-API gate, pre-change HEAD vs this tree): 28,463 of 28,464 files byte-IDENTICAL; the single diff is misc/diagnostics.html (cache-age readouts via unfrozen time.time - known noise class). misc/authorities.html rendered byte-identical this pair (probe latencies happened to match). pytest 1017. An earlier wall-clock gate showed 615 diffs - forensically attributed to two relays crossing the 7-day downtime filter between runs plus an hour-boundary 'ago' wave; the frozen-time gate eliminates both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The adversarial audit of the campaign confirmed a regression in Bug Phase 9's errno restriction: ssl.SSLError/SSLEOFError are OSError subclasses whose .errno holds OpenSSL error codes (1-10), never system errnos, so the _RETRYABLE_ERRNOS gate silently declassified transient TLS handshake failures as non-retryable. All five API endpoints are HTTPS: one handshake EOF on a stale-cache run would abort the entire generation where master retried with backoff (proven by live repro - a TCP listener closing mid-handshake). _is_retryable_error now classifies ssl.SSLError (bare or as URLError.reason) as retryable BEFORE the errno check, excluding SSLCertVerificationError (retries cannot fix a bad certificate). errno.EPROTOTYPE added for Darwin's send()-after-peer-reset quirk. 3 regression tests added (SSLEOFError retryable bare+wrapped, SSLCertVerificationError not, EPROTOTYPE retryable). Frozen-time gate: byte-identical except known diagnostics noise; pytest 1020. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
generate_search_index's parallel path merged per-batch lookup dicts via as_completed, so as_names/country_names key insertion order - and the serialized JSON - depended on thread completion order (reproduced 4/4 byte-differing outputs on identical input during the campaign audit). This polluted byte-level regression diffs of generated site trees. Two-layer fix: batches now merge in submission order (root cause), and the two lookup maps are sorted at serialization (canonical bytes regardless of merge or relay input order; platforms/flags were already sorted). JSON maps are order-insensitive to the search frontend. Frozen-time gate: 28,462/28,464 files byte-identical; the only non-noise diff is search-index.json's one-time canonicalization, verified semantically equal to the old output. New regression tests: two parallel runs over a >PARALLEL_THRESHOLD dataset assert byte equality (fails on old code), and parallel/sequential outputs assert agreement. pytest 1022. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The deselected (-m 'not slow') suite had accumulated years of rot and
could not run: 13 tests hung ~20 min each then failed, others made
live network calls or read/wrote the production cache. Root causes:
1. Eternal-read mocks: MagicMock read() returned the same non-empty
bytes forever, but _fetch_url_with_total_timeout reads chunks until
an EMPTY read - tests spun until total timeout (300-1200s) x retries.
Fixed at 9 sites + the shared fixtures helper with
read.side_effect=[payload, b'']; multi-worker tests get a fresh
response per urlopen call (a shared side_effect list exhausts after
the first worker).
2. Stale patch paths: 22 patch('lib.*') targets predate the allium.lib
layout - ModuleNotFoundError on every test in test_full_workflow.
3. Hermetic gaps: patching workers.CACHE_DIR does not rebind the
module-level _cache_manager/_timestamp_manager (bound at import), so
tests read/wrote the REAL allium/data/cache - cross-test pollution
and If-Modified-Since leakage. Rebound both at all 12 sites.
4. Ancient contracts: create_relay_set_with_coordinator kwargs
signature (takes an args namespace), fixtures lacking running:true
(dropped by the downtime filter), invented uptime document shape,
pre-consolidation log wording, and no-mock live-network placeholder
tests - all modernized with intent preserved and comments.
Results: the 3 affected files 83/83 passed; FULL suite with -m ''
1077 passed 0 failed in 5:03 (incl. live-endpoint system tests);
default selection unchanged at 1022 passed. Production code untouched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Security review: no high-confidence vulnerabilities found in this PR.
I focused on the changed trust boundaries in the diff: Onionoo/AROI/Exit DNS data flowing into Jinja templates, pre-rendered |safe HTML fragments, generated path components and vanity URLs, CLI-controlled fetch URLs/base URL, cache/state file handling, and the updated HTTP retry/fallback worker code. The changed code appears to preserve or add relevant controls (Jinja autoescape remains enabled, untrusted relay fields are escaped or JSON-serialized, path components are sanitized before filesystem writes, non-HTTP(S) fetch URL schemes are rejected, and scheme-relative base URLs are rejected).
No confirmed injection, XSS, path traversal, SSRF, auth boundary, secret handling, unsafe deserialization, or dependency/supply-chain vulnerability was identified from the diff.
Sent by Cursor Automation: Find vulnerabilities
cursor Bot
pushed a commit
that referenced
this pull request
Jul 10, 2026
Master's simplification refactor (#213–#215) removed the filter re-exports from relays.py (determine_unit_filter et al. now live in bandwidth_formatter/time_utils) and dropped the ip_utils import my fix relied on. CI on the PR merge ref failed with NameError: determine_ipv6_support / ImportError: determine_unit_filter. - relays.py: import determine_ipv6_support from ip_utils on the merged import line. - test_contact_template.py (TestContactTemplateIPv6Column.setUp): import filters from bandwidth_formatter/time_utils, matching the updated convention used by the rest of the file. Full suite on merged tree: 1038 passed. Co-authored-by: 1aeo <1aeo@users.noreply.github.com>
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
2 participants
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.
Implements the simplification plan from #211 (all 9 phases — fewer layers, fewer parallel ways of doing the same thing, byte-identical output), plus the campaign's adversarial-audit fix and a full restoration of the slow-marked test suite.
Phases
escape_relay_fields,safe_html_escape), 411 → 101 lines.handle_http_errors's 304/HTTP/network handling moved intoworkers._fetch_with_cache_fallback; per-API semantics live onAPIConfig(allow_exit_on_304/critical). 55-scenario error-path parity harness byte-identical (one intentional fix: bandwidth errors now route through the real progress logger — the old positional-args inspection could never find it).coordinator.API_WORKER_REGISTRYdeleted;workers.API_CONFIGSis the single source (worker_fn/args_fn/group/progress_name/feature_flag), with the CollecTor pair as real entries and the planned consensus-health path registered but dormant (enabled=False) per the plan decision.ProgressLoggerthreaded end-to-end; coordinator wrapper trio and manual step bookkeeping deleted. Progress output byte-identical (details-mode smoke, timestamps stripped).build_template_argsas the parallel workers; authority TCP probing moved out of rendering into the coordinator. Frozen-time gate: 28,463/28,464 files byte-identical (the one diff is the known unfrozen-clock diagnostics readout).evaluate_overload(2,352 boundary-fixture comparisons identical; caller semantic differences parameterized, not normalized), time_utils reuse, one shared url-token regex.misc_listing_macros.htmlextraction (5 misc pages 876→428 lines, 170 render-equality proofs) and oneoperator_hrefmacro. Two plan items refuted with evidence and left unchanged: relay-list/contact-relay-list share (tab- vs space-indented literals can't emit identical bytes from one macro) and the country.html summary merge (visible tooltip text differs — merging would silently change content).TemplateContextBuilderflattened (page_context 330 → 235 lines).Also in this PR
SSLError.errnoholds OpenSSL codes, not system errnos), so one handshake EOF on a stale-cache run would abort generation. Fixed (cert-verification errors stay non-retryable) with a live-repro-verified test.as_completed-ordered; reproduced 4/4 byte-differing outputs on identical input).-m slowtests had eternal-read mocks (spinning 20+ min then failing), stalelib.patch paths, and hermetic gaps writing to the real cache dir. All repaired: full suite with-m ""now 1,077 passed, 0 failed (was unable to complete); default selection unchanged.Verification
Every commit byte-gated (pinned APIs,
TZ=UTC,PYTHONHASHSEED=0); the riskiest phases additionally proven with a frozen-time harness giving full-tree byte-identity. Suite: 1,022 passing in default selection, 1,077 with slow tests included.Part 3 of the stack — based on the bug-fixes branch; merge last.
🤖 Generated with Claude Code