# Quality Batch — Implementation Plan (post-v7.34.0)

Deferred items from the v7.33.0 SWE/QA audit. Same process as the security batch: PR-only, Sonnet codes, Opus reviews (reads diff on disk + re-runs full suite + live-verifies where applicable), coder ≠ reviewer, per-PR full `pytest` gate. Target release: **v7.35.0** after all merge.

File sets mostly disjoint, but QB1 + QB2 both touch `canvas_api.py` → run **sequentially** (QB1 then QB2) to avoid conflicts. Order: QB1 → QB2 → QB5 → QB3 → QB4 → QB6.

---

## QB1 — Robustness + dead code (`canvas_api.py`, `audit_image_alt.py`, delete `push_bridge_handler.py`)

1. **Upload-path timeouts (B3/S10).** These raw `requests` calls bypass `_request_with_retry` and have NO timeout → can hang forever on a stalled socket: `canvas_api.py:309, 330` (upload_file POSTs), `:348` (confirm GET), `:558` (get_or_create_folder), `:570` (get_media_entry_id), `:590, 604` (upload_caption GET/PUT). Add `timeout=30` to each (longer is fine for the large-file POST at `:330` — use `timeout=120` there). **Do NOT route these through `_request_with_retry`** — the multi-step upload POSTs are non-idempotent; a retry after a timed-out-but-server-succeeded POST could create duplicate Canvas files. Timeouts only.
2. **Narrow the swallow.** `audit_image_alt._get`: `except (ValueError, Exception)` is effectively bare-`except` and would mask programming errors as `[]`. Narrow to `except (ValueError, requests.RequestException)`.
3. **Delete dead code.** `scripts/push_bridge_handler.py` — confirmed zero functional references (only self-refs; the `.push-request.json` it polls is written by nothing; added in 88a9515, never wired into any skill/CLAUDE/hook). Delete it (and any test referencing it — none found; re-grep to confirm before deleting).

**Deferred within batch:** `get_config`/`require_course_id` `sys.exit`→typed-raise (B2). Blast radius is ~70 CLIs that rely on the clean exit message; raising would regress their UX to tracebacks. Revisit as a dedicated change with a `get_config_or_exit()` CLI wrapper if we want it.

**Acceptance:** every listed upload call has a timeout; audit_image_alt except narrowed; push_bridge_handler.py gone; full suite green.

---

## QB2 — Dedup clusters (behavior-preserving refactor)

1. **`strip_html` ×11 → one `scripts/html_utils.py`.** Copies in: canvas_api, preflight_checks, deterministic_checks, alignment_validator, alignment_skill, alignment_graph_preview, audit_mlo_drift, audit_rubrics, audit_conversion_readiness, extract_course_layout, course_content_cache. The audit noted entity-handling DIVERGES across copies — pick the most-correct implementation, but **verify no existing test changes output**; if a call site depended on a quirk, preserve its semantics (add a flag rather than silently changing behavior). Replace all 11 with the shared helper.
2. **`load_course_config` ×9 → one shared loader.** Copies in: audit_bloom_alignment, audit_clo_orphans, audit_mlo_drift, audit_mlo_overclaim, audit_rubrics, audit_conversion_readiness, scope_triage, alignment_skill, alignment_graph_preview. Signatures diverge (some return `(dict, Path)`, some `dict`). Provide ONE helper with a documented return shape; adapt each call site. Preserve the `courses/{id}/course-config.json` → `./course-config.json` fallback exactly.
3. **Collapse literal dupes in `canvas_api.py`:** `get_all_pages`/`list_pages` (byte-identical) and `get_modules(include_items=True)`/`list_modules`. Keep both names as thin aliases if any caller uses each, or consolidate — but do NOT break the public call surface.

**Acceptance:** all copies replaced; full suite green with NO output changes (this is pure refactor — any diff in audit findings means a behavior change slipped in).

---

## QB5 — Test-gap backfill (no production code change)
Add tests for the untested critical scripts: `setup_env` (registry sync + `.env` auto-migration — corrupting `registry.json` breaks the Setup Gate), `agent_review_and_merge` (the merge gate itself), `course_content_cache`, `link_validator`. Mock the HTTP/FS layers. Goal: lock current behavior before future refactors touch them.

---

## QB3 — N+1 page-body fetches (E3)
Three loops fetch a pages index then GET each page body individually (each paying the inter-request overhead): `criterion_evaluator.py:157,170-175`, `course_content_cache.py:82-92`, `audit_pages.py:164-196` (which adds its own `time.sleep(0.15)`/page). **Reviewer verifies `GET /courses/{id}/pages?include[]=body` returns bodies on canvas.asu.edu first** (one live call). If yes → use the index `include[]=body` and drop the per-page GETs. If no → `ThreadPoolExecutor(max_workers=4-8)` for the body fetches + the new `requests.Session`. Either way, drop the manual `time.sleep` in audit_pages. Verify identical content vs the old path on a real course.

---

## QB4 — audit_pages WCAG fork (C4)
`audit_pages.py:49-68` keeps a pre-`wcag_checks` fork (own `_relative_luminance`/`contrast_ratio`/`AccessibilityAuditor`) → "audit" and "preflight" can disagree on the same page. Rebase `audit_pages` onto the shared `wcag_checks.WCAGContentParser`. Verify findings on a known fixture match the preflight engine.

---

## QB6 — Staging per-session CSRF/auth token (remaining M3 piece)
PR2 closed the M3 **read-exfil** vector (Host-header validation). The remaining gap: the staging server's mutating endpoints (PUT/DELETE) are gated only by an Origin allowlist — no per-session secret. **Design decision required before coding** (reviewer surfaces options to user): (a) token in a URL query param the launcher prints, checked on every mutating request; (b) token in a custom header the unified-review JS attaches, checked server-side; (c) token file under `courses/` the server reads. Trade-offs: URL-token leaks via referrer/history; header-token needs the review page to know it; file-token is simplest but local-FS-readable. Default lean: (b) header token minted at server start, embedded in served review HTML, required on PUT/DELETE. Hold until the user picks.

---

## Release
After QB1–QB5 (QB6 optional/separate): bump VERSION + both manifests to **7.35.0**, CHANGELOG section, tag. Coda sync auto-fires; engineer re-attaches zip.
