AUR-382 + AUR-393: Phase 3 complexity refactors + raidbots provider CLI - #32
Merged
Conversation
Extract behavior-preserving helpers to clear the two F-rated hotspots in warcraft_content/article_bundle.py: - query_article_bundle (F 53 -> A): per-kind haystack builders plus a generic `_collect_kind_matches`; section title-filter becomes a predicate closure. `results_by_kind` insertion order preserved (compact emit is order-sensitive). - compare_article_bundles (F 43 -> A): `_collect_bundle_evidence` single-pass accumulator plus `_build_analysis_surface_rows` / `_build_section_evidence_rows` / `_build_build_reference_rows`. Membership key-type asymmetry (build keyed by "::".join, others raw) and the identity-representative pick are kept verbatim. No output-shape change. radon: no E/F (or D) functions remain in the module. make check green (938 passed). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Behavior-preserving extraction of the two wrapper main.py E/F hotspots (same provider family, same file): - _guide_builds_simc_payload (F 77 -> C 11): split the per-build loop body into _build_simc_handoff_row (owns temp-packet create + try/finally unlink), with _resolve_handoff_build_code, _build_handoff_transport_packet, _invoke_simc_for_handoff, _handoff_evidence_section, _handoff_simc_section, _count_simc_handoff_successes, and _handoff_citations. - guide_compare_query (E 35 -> C 15): extract the per-provider loop body into _process_guide_compare_provider returning (provider_row, bundle_input|None), with _resolve_guide_compare_candidate for the resolve+search-fallback step. The loop still iterates selected_providers in order, appending provider_row always and bundle_input when present (order into compare_article_bundles and provider_results preserved). No output-shape change. radon: no E/F (or D) functions remain in wrapper main.py. make check green (938 passed). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clear the last two E-rated hotspots in wowhead_cli/citation_pack.py with behavior-preserving section collectors that keep delegating to _add_source / _add_claim (so the build_citation_pack sort+dedup winner and the #-fragment URL rewrite are unchanged): - citation_pack_from_entity (E 39 -> A): _entity_page_url + _collect_entity_page_and_citation_sources / _collect_tooltip_summary_claims / _collect_linked_entity_citations / _collect_comment_citations. - citation_pack_from_compare (E 38 -> A): _merge_entity_citation_packs / _collect_comparison_linked_entity_sources / _collect_comparison_field_claims. Verified byte-identical to the prior implementation via a differential check over entity edge cases and a full compare payload. Adds a snapshot test for citation_pack_from_compare (previously untested) covering the comment-key collision and #-fragment anchor rewrite. radon: no E/F (or D) functions remain in the module; full repo now has zero E/F. make check green (939 passed). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Behavior-preserving extraction of the five D-rated payload/resolver helpers in warcraftlogs_cli/main.py (follows the #31 boss_kills extraction style): - _resolve_encounter (D 25 -> A 5): split into _resolve_encounter_by_id / _resolve_encounter_by_name dispatch. - _encounter_ranking_row_payload (D 26 -> C 12): collapse the repeated first-non-empty/first-int `next(...)` scans into _first_non_empty_str / _first_int. - _boss_spec_usage_payload (D 22 -> A 4): extract _accumulate_boss_spec_counts. - _comp_samples_payload (D 26 -> A 4): extract _accumulate_comp_presence plus _record_comp_class_presence / _record_comp_signature (avoids relocating the hotspot into the accumulator). - _aura_compare_rows (D 22 -> B 10): extract the per-key _compare_row builder. No output-shape change. The only remaining D in the module is the pre-existing auth_pkce_login OAuth flow (out of scope for this analytics-payload slice). make check green (939 passed). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Behavior-preserving extraction of the simc D-rated build hotspots: build_input.py (all three D-rated functions cleared): - extract_build_spec_from_packet (D 28 -> A): one helper per transport form (_packet_spec_from_wowhead_ref / _wow_export / _split_talents), tried in the original short-circuit order so only the matching branch mutates source_notes. - extract_build_spec_from_text (D 23 -> A): _single_line_build_spec (keeps the detect()-set source_kind on the export path) + _parse_simc_build_text_lines. - identify_build (D 29 -> low): _direct_build_identity + _probe_build_matches. main.py: - modify_build_command (D 30 -> B 6): _resolve_modify_tree_entries / _build_modify_overrides / _assemble_modified_spec / _modify_build_diff_payload. modifications-list order and the no-modifications failure are preserved (_fail raises typer.Exit). Adds the BuildResolution import. No output-shape change. make check green (939 passed). The remaining simc main.py D is validate_talent_transport_command (separate command, outside this slice). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address a diffwarden finding (false positive) that read the extracted _resolve_modify_tree_entries as continuing after a swap-source decode failure. _fail raises typer.Exit, so the command aborts exactly as before; the post-_fail returns are unreachable defensive code (now noted inline). Adds a regression test proving a failing --swap-*-tree-from source aborts with decode_failed and emits no modified build even when --add is also present. make check green (940 passed). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark phase-3 complexity refactors complete (repo has zero E/F functions), note the two deliberately-deferred behavior-sensitive D functions (auth_pkce_login, validate_talent_transport_command), and record the behavior-preserving extraction work under CHANGELOG [Unreleased]. Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…R-393) New `raidbots-cli` package that consumes public Raidbots reports and bridges their SimC input to the local `simc` tool. No submission automation (no sanctioned Raidbots API) and no cross-provider import — handoff is by emitted commands, matching the wrapper's existing guide->simc design. Commands: doctor; inspect-report <url-or-id> (--no-raw); input <url-or-id>; explain-input (--text/--file/stdin). Reports are parsed from SimC json2 (quick-sim actor + metrics, or ranked profilesets for Top Gear/Droptimizer) with freshness/citations/scope; the handoff emits ready-to-paste input plus suggested local simc commands (shell-quoted, class/spec-resolved). Wired into the wrapper (provider registry + raidbots passthrough), import contracts (root + independence + layers), root scripts/hatch/mypy_path, conftest src paths + live-env map, packaged at the workspace version 0.3.0. Network/transport and parse failures map to the shared error envelope; report references normalize to an ID and fetch via the configured (env-overridable) base+templates, and the cache key includes the resolved URL. Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark the Raidbots "First Useful Slice" as shipped: a commands section in docs/raidbots/README.md (doctor / inspect-report / input / explain-input, handoff posture, env-overridable URLs, deferred submission), promote it from "planned" in the docs index, and add consumer-facing routing to the warcraft skill (provider synopsis row, Read Next entry, references/raidbots.md). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mark Tier 1/2 and the First Useful Slice as shipped (linking the authoritative Commands section), and note the local-vs-report crosswalk stays deferred — the no-import handoff covers the common case. Keeps the doc consistent with actual CLI behavior. Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ors) Combine the two independent feature branches so the PR carries both the behavior-preserving complexity refactors and the new raidbots provider CLI. Branches were siblings off main with non-overlapping changes (raidbots package + wrapper wiring vs. helper extractions); .gitignore/CHANGELOG additions are disjoint. Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… APL need Post-merge fixes surfaced by the combined-PR diffwarden review and merge-readiness audit: - scripts/dev_deploy.sh: add `raidbots` to BIN_NAMES so `make dev-deploy` links the console script alongside the other eight providers (it was the only one omitted). - Makefile: add RAIDBOTS_LIVE_TESTS=1 to LIVE_TEST_ENV so `make test-live` actually reaches the raidbots live tests instead of silently skipping them on the env gate. - raidbots simc_input: the suggested `simc describe-build` command now carries a `requires` hint — unlike decode-build it needs a default/explicit APL and otherwise fails not_found when no SimC repo is checked out. Add a regression test asserting the hint is present on describe-build and absent on the self-sufficient decode-build. Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion)
A combined-PR diffwarden run flagged (2 reviewers, P2) that
`_collect_comparison_linked_entity_sources` "drops unique_by_entity URLs"
because the visible loop only iterates the single-element ("shared_items",)
tuple. It does not: the dict-shaped unique_by_entity bucket is collected by
the loop immediately below. Verified by differential execution — the slice-4
extraction's compare output is byte-identical to the pre-refactor builder on a
payload populating both shared_items and unique_by_entity, and the existing
compare snapshot test already asserts the unique_by_entity keys.
Add an inline comment documenting the two-shape split so future reviews don't
re-fixate on the apparent (refuted) drop. No behavior change.
Co-Authored-By: OpenAI Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aurokin
changed the title
…tract
From a second combined-PR diffwarden run (two single-reviewer P2s, both verified):
- Fix (codex-gpt55, valid): SimC accepts both the addon/profile actor form
(`deathknight=`) and the documented manual-creation keyword form
(`death_knight=`/`demon_hunter=`). `_find_actor` only matched the former, so a
report whose SimC input used the underscore form was misclassified as `advanced`
and dropped the decode/describe handoff. Normalize the class token (strip
non-alphanumerics, mirroring simc-cli's _normalize_actor_class) before the class
check and emit the canonical no-underscore form. Verified against SimC's own
midnight-branch profiles (which use `deathknight=`) and the upstream class-keyword
docs. Added a regression test for both DK and DH underscore tokens.
- Decline (codex-gpt54, by-design): `resolve_report_id` pins URL-*input* parsing to
the documented `/report/{ID}` shape and always accepts a bare ID. Env overrides
adapt the *fetch* URLs to live drift with no code change; a bare ID is drift-proof.
Sharpened the inline comment to state this distinction so future reviews don't
re-fixate. No behavior change.
make check green (963 passed, 237 deselected; ruff/mypy/import-linter clean).
Co-Authored-By: OpenAI Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Final-round diffwarden finding (codex-gpt54, conf 0.94, verified valid): `_iter_clean_lines` dropped comment-only lines but preserved trailing `# ...` comments, so `mage="Main" # actor` missed the actor entirely (the actor regex anchors on end-of-line) and `spec=frost # note` / `talents=CYG # build` carried the comment into the suggested decode/describe commands. SimC treats `#` as a comment delimiter anywhere on a line; strip it like simc-cli's build-text parser (build_input.py:330,370). Added a regression test over a fully commented profile. make check green (964 passed, 237 deselected). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…o_talents) A scoped execution-based classifier audit (5 construct categories, adversarially verified) surfaced one confirmed realistic in-scope gap: profiles whose talents are expressed via the split keys (class_talents=/spec_talents=/hero_talents=) with no combined talents= line yielded talents_present=False, so simc_handoff emitted only `simc sim -` and suppressed the decode/describe handoff — even though actor_class and spec were extracted. simc-cli treats this form as first-class (simc_split_talents), and simc decode-build/describe-build already accept --class-talents/--spec-talents/ --hero-talents (verified via source + runtime --help). Recognize the split keys: classify_simc_input now reports talents_present for the split form, and simc_handoff prefers the combined talents= line but falls back to the split flags (each shlex.quote'd) so the decode/describe commands still fire. Combined form still wins when both are present (canonical addon-export shape). Added regression tests for the split-only and combined+split cases. make check green (966 passed, 237 deselected). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ce-padded talents
Three verified findings from the latest combined-PR diffwarden run (codex-gpt54;
codex-gpt55 + cursor-composer clean):
- URL round-trip (conf 0.96): doctor/citations build report URLs from the env-overridable
RAIDBOTS_REPORT_PATH_TEMPLATE, but resolve_report_id only extracted IDs from the literal
`/report/` shape, so an overridden template emitted URLs the CLI's own input parser would
reject. resolve_report_id now derives the extraction prefix from the configured template
(single source of truth), keeping the `/report/{ID}` and bare-ID fallbacks and the
host-ignoring SSRF invariant. _resolve_report_id_or_fail threads the configured template.
- Profileset presence (conf 0.91): `_has_profilesets` keyed on non-emptiness, so a bare
`"profilesets": []` fell through to the quick-sim path and reported players[0] (the
multi-profile baseline template) as a user actor — contradicting the function's own intent.
Now routes on container presence; quick sims never emit the key. Added an empty-list test.
- Space-padded assignments (conf 0.93): talents detection used exact `talents=`/`class_talents=`
prefixes while _scalar_assignments and _find_actor already tolerate whitespace around `=`, so
`talents = CYG` silently dropped the decode/describe handoff. talents/_split_talents now split
on the first `=` and strip the key; talents_present derives from the parsed assignments.
make check green (969 passed, 237 deselected; ruff/mypy/import-linter clean).
Co-Authored-By: OpenAI Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nce) Follow-on to the previous space-padding fix (codex-gpt54, conf 0.97): copy_count still used an exact `copy=` prefix while talents/actor/assignments tolerate whitespace, so `copy = alt` (valid SimC) misclassified a multi-profile run as quick_sim/advanced. Count copy directives via the same split-on-first-`=` + stripped-key rule (guarded on `=` presence so a bare `copy` line is not counted). Completes the classifier's space-tolerance. make check green (970 passed, 237 deselected). Co-Authored-By: OpenAI Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ing)
diffwarden (codex-gpt54) flagged that a `RAIDBOTS_REPORT_PATH_TEMPLATE='/{id}'`
root-level override would stop URL round-tripping because resolve_report_id only
uses the template prefix when it is longer than "/". Declined: that guard is
deliberate — a root/empty prefix would mis-extract the first path segment of any
URL, and reports are not served at a root-level `/{id}` on Raidbots. Documented the
rationale inline so the guard isn't relaxed into a mis-extraction regression. No
behavior change. Bare IDs always resolve regardless of template.
Co-Authored-By: OpenAI Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
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.
Closes AUR-382 (phase 3 complexity refactors) and AUR-393 (Raidbots report consumption + local SimC handoff).
AUR-382 — Phase 3 complexity refactors (behavior-preserving)
Helper/module extraction to clear every Radon E/F hotspot and the named D targets. No output shapes change.
warcraft_content.article_bundle:query_article_bundleF53→A,compare_article_bundlesF43→A.main.py:_guide_builds_simc_payloadF77→C,guide_compare_queryE35→C.wowhead_cli.citation_pack:citation_pack_from_entityE39→A,citation_pack_from_compareE38→A (differential byte-identical; compare snapshot test added).main.py: five D-rated analytics payload/resolver helpers → ≤C.build_input.py+modify_build_commandD30→B6; swap-decode abort regression test.radon cc -n E packages/is now empty. Remaining D functions left intentionally (behavior-sensitive, not analytics payloads):warcraftlogs auth_pkce_login,simc validate_talent_transport_command.AUR-393 — Raidbots provider CLI
New
raidbots-cli(raidbots …directly orwarcraft raidbots …). Consumes completed, public Raidbots reports and hands off to localsimc; no submission automation (no sanctioned API) and no cross-provider import.doctor,inspect-report <url-or-id>(--no-raw),input <url-or-id>,explain-input(--text/--file/stdin).json2parsing (quick-sim actor + metrics vs ranked profilesets for Top Gear/Droptimizer); freshness/citations/scope.simccommands (shlex-quoted); the classifier recognizes bothdeathknight/death_knightclass forms, trailing#comments, splitclass/spec/hero_talents, and space-padded assignments.warcraftdist (hatch packages), wrapper capabilities (search/resolvenot_supported, retail expansion),.importlinter,scripts/dev_deploy.sh, andmake test-live.See
docs/raidbots/README.md.Combined verification
make checkgreen: 970 passed, 237 deselected; ruff + mypy + import-linter all clean.main.codex-gpt54/codex-gpt55/cursor-composer) over the combinedbase:maindiff, iterated to convergence: codex-gpt55 + cursor-composer clean. Every valid finding fixed with a regression test — deploy/live-test wiring, underscore class tokens, trailing#comments, split talents, empty-list profilesets, space-padded talents/copy, env-override URL round-trip. False positives (root editable install; compareunique_by_entitydrop) were refuted by execution and annotated inline; one pathological/{id}URL-template edge was declined by design (guard documented inline). A scoped, execution-based classifier audit surfaced the split-talents gap proactively.🤖 Generated with Claude Code