Phoenix Rising corpus: LDN & Mestinon scraper, analysis scripts, S3 data upload - #41
Phoenix Rising corpus: LDN & Mestinon scraper, analysis scripts, S3 data upload#41Ely-S wants to merge 11 commits into
Conversation
- Scrapers/scrape_phoenixrising.py: robots-compliant XenForo scraper emitting the
PatientPunk corpus schema (thread->post, replies->comments, quote->parent linkage).
- Scrapers/phoenixrising_targets.txt: 276 drug-focused thread URLs (sitemap-derived).
- drugs/{naltrexone,pyridostigmine}.txt: curated alias lists.
- scripts/: free use-counts, sample + full-census classification export, aggregation
with gold-set validation.
- docs/phoenixrising_use_stats.md: write-up (use, dosing, barriers, sentiment).
- pyproject: add beautifulsoup4 (scraper dependency).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Raw real-world-evidence data backing the LDN/Mestinon analysis:
- output/phoenixrising_posts.json : full scraped corpus (276 threads, ~5,081 posts)
- data/phoenixrising.db : SQLite with posts imported (ready for scripts)
- outputs/manual/ : 400 hand-labels + full-census agent labels,
per-batch inputs, census_summary.json, sentiment_summary.json
Usernames are SHA-256 hashed. Force-added past .gitignore intentionally (per maintainer decision).
(The 3-thread pr_test scaffold was left out as a redundant subset of the full corpus.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
External site-scoped web search surfaced 17 threads that discuss LDN/Mestinon in post bodies but not the title (recovery-story lists, treatment guides, dysautonomia threads, and a misspelled-title "Mestonin" thread). Adds 770 posts -> +46 LDN and +53 Mestinon new direct mentions; all classified (stragglers + labels_recall_stragglers.json). Combined corpus: 293 threads. Sentiment split essentially unchanged vs the title-based core (robustness check). docs/phoenixrising_use_stats.md and census_summary.json updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deterministic (no-AI) /tags/ crawl (ldn, naltrexone, mestinon, pyridostigmine) added 5 threads not already captured (+23 LDN, +3 Mestinon mentions; all classified). Final near-complete corpus: 298 threads — LDN 2,516 posts / 534 participants; pyridostigmine 389 / 121. Sentiment unchanged across all four expansions (LDN 38% positive / 63% some-benefit; Mestinon 42% / 60%), validation 84/90/93. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…unt print - docs/recall_search_provenance.md: records that the 17 body-mention threads were discovered by Claude via web search (the 17 exact queries listed), with an explicit AI-assisted / not-deterministically-reproducible caveat for the methods write-up. - free_use_counts.py: replace hardcoded "276 threads" with a computed thread count. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Handle missing/anonymous authors by mapping to stable "anon" user ID - Support non-Reddit forums via SOURCE_NAME_BY_HOST hostname mapping - Repair dangling parent links (forum quotes referencing unexported posts) - Add input validation for resumed imports and source detection - Fix robots.txt URL path parsing and thread URL normalization Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Migrate Phoenix Rising search discovery to Brave Search API with documented operator usage, split LDN/Mestinon query sets, and add a paginated analyzer that writes per-query outputs plus exact minimum set cover after manual curl validation and 429-aware retries. Co-authored-by: Cursor <cursoragent@cursor.com>
Code Review by Qodo
1. Robots redirect bypass
|
There was a problem hiding this comment.
Code Review
This pull request introduces a Python scraper (scrape_phoenixrising.py) designed to extract patient self-report threads from the Phoenix Rising ME/CFS forum, along with associated target lists, search queries, drug alias files, and documentation of real-world use statistics. Feedback on the scraper focuses on improving robustness: first, by wrapping sub-sitemap fetching in a try-except block to prevent a single network failure from crashing the entire sitemap discovery process; and second, by writing incrementally to a temporary file and atomically replacing the target output file to avoid potential file corruption during long-running operations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for sm in sub_sitemaps: | ||
| xml = fetch(sm, delay) | ||
| for loc in re.findall(r"<loc>([^<]+)</loc>", xml): | ||
| normalized = normalize_thread_url(loc) | ||
| if normalized: | ||
| all_threads.add(normalized) |
There was a problem hiding this comment.
If fetching or parsing a single sub-sitemap fails (e.g., due to a temporary network glitch or 500 error), the entire sitemap discovery process will crash. Wrapping the sub-sitemap processing in a try-except block ensures that a single failure does not prevent the discovery of threads from other healthy sub-sitemaps.
for sm in sub_sitemaps:\n try:\n xml = fetch(sm, delay)\n for loc in re.findall(r\"<loc>([^<]+)</loc>\", xml):\n normalized = normalize_thread_url(loc)\n if normalized:\n all_threads.add(normalized)\n except Exception as e:\n print(f\" ! failed to fetch sub-sitemap {sm}: {e}\", file=sys.stderr)| results.append(post) | ||
| total_comments += len(post["comments"]) | ||
| # Incremental, crash-safe write after every thread. | ||
| out_path.write_text(json.dumps(results, ensure_ascii=False, indent=2), encoding="utf-8") |
There was a problem hiding this comment.
Writing directly to the target output file (out_path) during long-running scraping operations poses a risk of file corruption if the process is interrupted (e.g., via Ctrl+C or a crash) mid-write. To ensure robustness, write the JSON to a temporary file first and then atomically replace the target file.
temp_path = out_path.with_suffix(\".tmp\")\n temp_path.write_text(json.dumps(results, ensure_ascii=False, indent=2), encoding=\"utf-8\")\n temp_path.replace(out_path)Keep Phoenix Rising and sample scrape outputs out of git by deleting tracked data dumps/logs and tightening ignore rules for regenerated files. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep Phoenix Rising and sample scrape outputs out of git by deleting tracked data dumps/logs and tightening ignore rules for regenerated files. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Superseded by a cleanup-only PR from branch cleanup/scraper-artifacts. |
Code Review by Qodo
1. Robots redirect bypass
|
PR Summary by QodoAdd Phoenix Rising scraper with Brave discovery and query set-cover tooling WalkthroughsDescription• Add robots-compliant Phoenix Rising XenForo scraper emitting PatientPunk corpus JSON. • Add Brave Search API discovery and query-set cover minimization to reduce queries. • Import scraped forum data into SQLite and publish LDN/Mestinon use/sentiment write-up. Diagramgraph TD
QFiles["Search query files"] --> Cover["Query cover tool"] --> Brave{{"Brave Search API"}} --> Cover
Cover --> MinQ["Min query files"] --> Scraper["Phoenix Rising scraper"] --> Posts["Scraped posts JSON"] --> Importer["DB importer"] --> DB[("SQLite DB")]
DB --> Analysis["Analysis scripts"] --> Docs["RWE docs"]
subgraph Legend
direction LR
_ext{{"External API"}} ~~~ _svc["Script/Module"] ~~~ _file["Data file"] ~~~ _db[("Database")]
end
High-Level AssessmentThe following are alternative approaches to this PR: 1. Use OR-Tools / ILP solver for set cover
2. Centralize Brave API client shared by scraper + tooling
3. Prefer sitemap-first, search as incremental recall-only mode
Recommendation: Current approach (exact set cover when small, greedy fallback; sitemap/thread-list options retained) is a good tradeoff for reliability and cost. If query sets grow beyond ~20–30 queries, consider introducing an ILP-based solver and/or a shared Brave client module to reduce duplicated logic. File ChangesEnhancement (7)
Bug fix (1)
Tests (1)
Documentation (2)
Other (126)
|
| def fetch(url: str, delay: float, retries: int = 5) -> str: | ||
| """GET a URL as text with polite delay and exponential backoff.""" | ||
| _check_robots(url) | ||
| last_exc = None | ||
| for attempt in range(retries): | ||
| try: | ||
| req = urllib.request.Request( | ||
| url, | ||
| headers={ | ||
| "User-Agent": UA, | ||
| "Accept": "text/html,application/xhtml+xml,application/xml", | ||
| "Accept-Encoding": "identity", | ||
| }, | ||
| ) | ||
| with urllib.request.urlopen(req, timeout=60) as resp: | ||
| raw = resp.read() | ||
| time.sleep(delay) |
There was a problem hiding this comment.
2. Robots redirect bypass 🐞 Bug ☼ Reliability
fetch() enforces _check_robots(url) only on the initially requested URL, but it does not validate the final URL after urlopen completes, so a redirect can lead to fetching a robots-disallowed path while still passing the guard. This contradicts the scraper’s stated guarantee that it “refuses any path in the robots.txt Disallow list.”
Agent Prompt
## Issue description
The scraper claims robots compliance via a guard, but `fetch()` only checks the input URL. If the server redirects (e.g. to `/login/`), the code can fetch disallowed content without ever re-checking robots constraints.
## Issue Context
`ROBOTS_DISALLOW` explicitly includes paths like `/login/` and `/search/`, and the module docstring asserts the guard prevents requesting any disallowed path.
## Fix Focus Areas
- Scrapers/scrape_phoenixrising.py[69-75]
- Scrapers/scrape_phoenixrising.py[87-128]
## Suggested fix
- Make redirect handling explicit and robots-aware:
- Option A (preferred): build a custom opener with an `HTTPRedirectHandler` override that calls `_check_robots(new_url)` inside the redirect handling before following it.
- Option B: disable redirects, manually handle 3xx by reading the `Location` header, calling `_check_robots(location)`, then issuing the next request.
- Apply the same approach anywhere you fetch Phoenix Rising HTML/XML (i.e., `fetch()`), so the “only /threads and /sitemap*.xml fetched” guarantee holds end-to-end.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| results.append(post) | ||
| total_comments += len(post["comments"]) | ||
| # Incremental, crash-safe write after every thread. | ||
| out_path.write_text(json.dumps(results, ensure_ascii=False, indent=2), encoding="utf-8") |
There was a problem hiding this comment.
3. Non-atomic output rewrites 🐞 Bug ☼ Reliability
The scraper rewrites the entire JSON output (json.dumps(results)) in-place after every thread and labels this as “crash-safe,” but an interruption mid-write can leave a truncated/invalid JSON file and break resume. This also creates quadratic I/O as results grows, slowing long scrapes significantly.
Agent Prompt
## Issue description
The main scrape loop overwrites the full output JSON file after each thread via `Path.write_text(...)`. This is not atomic, so crashes/interruptions can corrupt the output file, and it becomes increasingly expensive as the output grows.
## Issue Context
The code explicitly intends to be “Incremental, crash-safe,” but the current mechanism does not guarantee a valid file on partial writes.
## Fix Focus Areas
- Scrapers/scrape_phoenixrising.py[567-580]
## Suggested fix
- Make the checkpoint write atomic:
- Write to a temporary file in the same directory (e.g., `out_path.with_suffix('.tmp')`), flush/fsync, then `os.replace(tmp, out_path)`.
- Consider switching to an append-friendly format for checkpoints:
- JSONL per thread (plus a small metadata/index file), or
- periodic checkpoints (every N threads) instead of every thread.
- Ensure resume logic can tolerate partial state (e.g., if using JSONL, ignore a final incomplete line).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…outputs - scrape_phoenixrising.py: remove mutually exclusive source group; allow --from-search-api + --thread-list to combine; make both appendable for multiple files; add --url-out flag (collect URLs, write list, exit without scraping); accept multiple --search-query-file args; add flush=True to print calls for background-task visibility - drugs/naltrexone.txt, drugs/pyridostigmine.txt: canonical alias files now read by all four analysis scripts (aggregate_census, export_for_manual, export_full, free_use_counts) — single source of truth for alias sets - scripts/aggregate_census.py: replace hardcoded DB path with --db arg - outputs/manual/: remove 110 generated classification artifacts from version control; add outputs/manual/ to .gitignore Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Eli, sorry, completely missed this requested review. I'm not too worried about this bc of the one offness of this, but a few small things I caught through codex. |
Airwhale
left a comment
There was a problem hiding this comment.
Leaving a few comment-only notes. Most of these are small reliability or reproducibility edges rather than concerns about the overall one-off workflow.
| except Exception as e: # noqa: BLE001 | ||
| print(f" ! failed {url} page {n}: {e}", file=sys.stderr) | ||
| break |
There was a problem hiding this comment.
AI found:
A partially scraped multi-page thread as complete when a later page fails. Right now this breaks out of the page loop, but the caller still appends the thread to --out; on the next run, done_ids will skip that thread_id, so a transient page error can become a persistent undercount. Returning None here keeps the thread out of the completed set so a later resume can retry it.
| except Exception as e: # noqa: BLE001 | |
| print(f" ! failed {url} page {n}: {e}", file=sys.stderr) | |
| break | |
| except Exception as e: # noqa: BLE001 | |
| print(f" ! failed {url} page {n}: {e}", file=sys.stderr) | |
| return None |
| @@ -0,0 +1,394 @@ | |||
| [ | |||
There was a problem hiding this comment.
AI found: (Note that I don't really care that much if these are in the github: up to your judgment -SG)
I think these generated artifacts may have slipped in unintentionally. The PR description says generated data lives in S3 and only code/config are tracked, but this file and outputs/naltrexone/tagged_mentions.json contain derived scraped forum text plus author hashes. Since the new ignore rule only covers outputs/manual/, could we remove these two files from git and add a scoped ignore for the generated outputs/* paths that should remain local?
| aliases_by_label = {label: read_alias_file(path) for label, path in DEFAULT_ALIAS_FILES.items()} | ||
| conn = sqlite3.connect(args.db) | ||
| rows = conn.execute("SELECT post_id,title,parent_id,user_id,body_text FROM posts").fetchall() | ||
| conn.close() |
There was a problem hiding this comment.
AI found:
Small fresh-checkout guard: outputs/manual/ is ignored, so outputs/manual/full will not exist until someone has generated local classification files. Creating it before writing stragglers keeps the script from failing on the first run.
| aliases_by_label = {label: read_alias_file(path) for label, path in DEFAULT_ALIAS_FILES.items()} | |
| conn = sqlite3.connect(args.db) | |
| rows = conn.execute("SELECT post_id,title,parent_id,user_id,body_text FROM posts").fetchall() | |
| conn.close() | |
| FULL.mkdir(parents=True, exist_ok=True) | |
| aliases_by_label = {label: read_alias_file(path) for label, path in DEFAULT_ALIAS_FILES.items()} | |
| conn = sqlite3.connect(args.db) | |
| rows = conn.execute("SELECT post_id,title,parent_id,user_id,body_text FROM posts").fetchall() | |
| conn.close() |
| print(f"{label} — FULL CENSUS (final aliases)") | ||
| print("=" * 70) | ||
| print(f" Candidate posts (new aliases): {len(cand_ids)} | labeled: {n} | unlabeled stragglers: {len(stragglers)}") | ||
| print(f" Personal-experience posts : {exp} ({100*exp/n:.0f}% of labeled)") |
There was a problem hiding this comment.
AI found:
Related fresh-checkout edge: if no label files have been loaded yet, n can be zero here and this print path raises before writing the summary/stragglers. A small guard keeps the script usable before the classification artifacts exist.
| print(f" Personal-experience posts : {exp} ({100*exp/n:.0f}% of labeled)") | |
| pct_labeled = f"{100 * exp / n:.0f}%" if n else "n/a" | |
| print(f" Personal-experience posts : {exp} ({pct_labeled} of labeled)") |
| @@ -0,0 +1,160 @@ | |||
| # Phoenix Rising — real-world USE data for the FDA comment (Section 6 & 7) | |||
|
|
|||
| **Source:** Phoenix Rising ME/CFS Forums (forums.phoenixrising.me), 276 drug-focused | |||
There was a problem hiding this comment.
AI found:
Could we refresh this tracked write-up before merge? It appears to describe an earlier corpus snapshot (276 title-discovered threads, then 293/298 after recall expansion, 5,081 posts/comments), while the PR body's final reproduction path says the uploaded artifact is based on 316 input URLs, 314 imported threads, 6,657 posts/comments, and 853 participants. Keeping the committed methods/results doc aligned with the final artifact would avoid conflicting denominators later.
| def _fresh_db(path: Path) -> sqlite3.Connection: | ||
| conn = sqlite3.connect(path, check_same_thread=False) | ||
| conn.execute("PRAGMA foreign_keys = ON") | ||
| conn.executescript(SCHEMA.read_text()) |
There was a problem hiding this comment.
AI found:
Tiny portability fix: Path.read_text() uses the platform default encoding. On Windows this test fails before reaching the assertions because schema.sql contains UTF-8 box-drawing characters and the default codec is cp1252. Pinning the encoding keeps local runs consistent with CI/Linux.
| conn.executescript(SCHEMA.read_text()) | |
| conn.executescript(SCHEMA.read_text(encoding="utf-8")) |
|
Just a thought: maybe we should localize all of the different specific research work : this, the FDA stuff, the pilot, into a specific folder. |
Establish a root-level studies/ folder to hold all study methodology in one place, and relocate the IRR pilot into it as a sub-study of the RCT historical-validation study: docs/irr_pilot/ -> studies/rct_validation/irr_pilot/ Adds studies/README.md (the source-of-truth index of every study and where it belongs) and studies/rct_validation/README.md (context for the nesting). Updates the .gitignore sample-pack paths and one path reference in the codebook. Intended full layout, landed per-branch as each PR merges: studies/ ├── rct_validation/ (from docs/RCT_historical_validation/ on main) │ └── irr_pilot/ (this PR) ├── fda_letter/ (PR #52) │ └── phoenix_rising/ (PR #41) └── natural_exploration/ (TrialScout — not ready to merge) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Establish a root-level studies/ folder to hold all study methodology in one place, and relocate the IRR pilot into it as a sub-study of the RCT historical-validation study: docs/irr_pilot/ -> studies/rct_validation/irr_pilot/ Adds studies/README.md (the source-of-truth index of every study and where it belongs) and studies/rct_validation/README.md (context for the nesting). Updates the .gitignore sample-pack paths and one path reference in the codebook. Intended full layout, landed per-branch as each PR merges: studies/ ├── rct_validation/ (from docs/RCT_historical_validation/ on main) │ └── irr_pilot/ (this PR) ├── fda_letter/ (PR #52) │ └── phoenix_rising/ (PR #41) └── natural_exploration/ (TrialScout — not ready to merge) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Relocate the study into the root-level studies/ folder so all study methodology lives in one place: docs/FDA_letter_analysis/ -> studies/fda_letter/ Pure move — the build scripts use __file__-relative paths at the same directory depth (parents[1]=package, parents[3]=repo root both still resolve), so no code changes are needed. Updates the one external path reference in the root README index. Part of consolidating every study under studies/ (see PR #39). The Phoenix Rising corpus will nest here as studies/fda_letter/phoenix_rising/ once its PR (#41) lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What this PR does
Adds end-to-end infrastructure to scrape the Phoenix Rising ME/CFS forum, build a queryable SQLite corpus, and produce real-world-evidence statistics for LDN and pyridostigmine (Mestinon). All generated data is uploaded to S3; only code and configuration are tracked in git.
Corpus — produced data
Location:
s3://patientpunk/data/phoenixrising/phoenixrising.dbschema.sql)phoenixrising_posts.jsonphoenixrising_metadata.jsonall_pr_targets.txtREADME.mdCorpus statistics
Total: 6,657 posts/comments · 853 unique participants · 314 threads · Phoenix Rising ME/CFS forum · 2009–2026
Low-Dose Naltrexone (LDN)
Note: 50mg appears in 150 posts — full-dose naltrexone references, flagged but not excluded.
Pyridostigmine / Mestinon
Exact reproduction steps
Requires
uv, Python 3.13,BRAVE_SEARCH_API_KEYin environment.Step 1 — Collect URLs
Combines three sources into one deduplicated target list:
Sources merged:
phoenixrising_targets.txt— 276 hand-curated thread URLsphoenixrising_recall_targets.txt— 17 body-mention recall threadsphoenixrising_tag_targets.txt— 5 tag-page threadsResult: 316 unique thread URLs.
Step 2 — Scrape
Config: 2.5s polite delay, up to 500 pages/thread, usernames SHA-256 hashed before any disk write. Robots.txt compliant.
Result: 316 threads, 6,717 replies.
Step 3 — Import
sqlite3 data/phoenixrising.db < schema.sql uv run python src/import_posts.py \ --reddit-posts output/phoenixrising_posts.json \ --output-db data/phoenixrising.dbResult: 853 users, 6,657 posts/comments.
Step 4 — Statistics
Step 5 — Upload to S3
Code changes
Scrapers/scrape_phoenixrising.py— two-step collect→scrape architectureReplaced the mutually exclusive
--from-sitemap / --from-search-api / --thread-listsource group with composable flags:--from-search-apiand--thread-listcan now be combined in one invocation--thread-listand--search-query-fileare both appendable (action="append") — pass each file as a separate flag--url-out FILE— new flag: collect URLs from all sources, write deduplicated list, exit without scraping. Enables the clean two-step workflowdiscover_from_search_apinow acceptsquery_files: list[Path]instead of a single fileflush=Trueto all progressprint()calls so background tasks produce live outputdrugs/naltrexone.txt/drugs/pyridostigmine.txt— canonical alias filesSingle source of truth for alias sets, read by all four analysis scripts. Previously each script had its own hardcoded list; they had silently diverged (e.g.
export_for_manual_classification.pyincluded"revia"/"vivitrol"whichaggregate_census.pyintentionally excluded).scripts/aggregate_census.pyReplaced hardcoded
DB = "data/phoenixrising.db"with--dbCLI arg, consistent with the other three scripts.scripts/export_for_manual_classification.py/export_full_for_classification.py/free_use_counts.pyUpdated to read alias sets from
drugs/*.txtviaread_alias_file()(was already done on disk for these three; this commit locks it in and removes the old inline lists)..gitignoreAdded
outputs/manual/— 110 generated classification batch files (JSONL + JSON label outputs) were previously tracked; removed from index viagit rm --cached -r.