Skip to content

Add primer-guided deck improvements and fix commander/import reliability - #85

Open
LlamaAdam wants to merge 10 commits into
masterfrom
feat/fp-019-primer-heuristics
Open

Add primer-guided deck improvements and fix commander/import reliability#85
LlamaAdam wants to merge 10 commits into
masterfrom
feat/fp-019-primer-heuristics

Conversation

@LlamaAdam

Copy link
Copy Markdown
Owner

Summary

  • Add the approved FP-019 primer-guided improvements: bundled knowledge base, consistency floors and dashboard targets, context-aware roles and quotas, ranking penalties, self-conflict checks, and grounded advisor/judge/budget guidance.
  • Harden primer evidence: current strategy is separated from historical/page noise, links are references rather than automatic cut protection, quantities are counted correctly, and card presence is not presented as rules verification.
  • Fix all six negative-mode findings: honest legal/illegal/unverified results; preserved import sections and foil/etched normalization; straightforward commander/partner selection; rejected empty/malformed imports; shared saved-library resolution; isolated Windows-compatible tests.
  • Guard late import/commander/simulation responses against stale navigation and preserve keyboard focus. Rules details retain affected card names and data-age warnings.
  • Require explicit --run-live consent for real-service tests, independent of slow-test selection.

Verification

  • Full Python regression lane (--run-slow): 4,590 passed, 1 skipped in 636.85 seconds. The sole skip is the explicit live-service opt-in; all offline slow tests ran. Existing positional-maxsplit deprecation warnings remain.
  • Playwright: 26 passed, including the user's complete 100-card Ur-Dragon export, partner changes, invalid imports, delayed saves/navigation, and simulation invalidation.
  • Manual browser smoke in a disposable library: commander changes preserve quantities; absent commanders warn before leaving an oversized deck; persistence, rules details, and focus restoration work.
  • Wheel build and packaged editor/data/assets verified; Python compilation, JavaScript syntax, whitespace checks, and staged secret scan passed.
  • Independent backend, Python, and frontend reviews completed; review findings reproduced and fixed with regression tests.

Limits

The manual/UI tests use stub card data and simulation reports. No live Forge gameplay or successful model-service request is claimed. The initial full run exposed a legacy live-Claude test missing its optional SDK (4,582 other tests passed); the live lane now requires an explicit opt-in and configured optional dependencies. User deck libraries and raw primer harvests were not modified.

See docs/negative-mode-audit-2026-09-01.md for the original findings and resolution details. This PR includes the nine existing FP-019 commits; it does not merge or deploy them.

40 community primers (21 Moxfield + 19 Archidekt, harvested 2026-08-29)
distilled to per-deck structured records, shipped as package data with a
fail-quiet, immutable, offline loader. Profiles-not-one-truth (§13):
profiles_for_commander returns every build of a commander; win-line card
names carry per-card mainboard-verification flags; budget swaps flatten
into a §10 function-preserving swap corpus; prompt_block_for_commander
renders clipped LLM context via primer.clip_for_prompt.
CONSISTENCY_TARGETS names the section-1 numbers that converged across
independent primer authors (85% third land drop, 85% commander on
curve, 90% card advantage by T5, the 13-enabler free-mulligan rule,
at most 2 unconditionally-tapped fetchables) and
evaluate_consistency_targets grades a deck against them, reusing the
Monte-Carlo projection deck_health already computes plus closed-form
hypergeometrics. Conditional floors (t1_enabler_plan /
proactive_t2_plan) report their value but stay unjudged until a caller
declares the plan. Wired as an additive-only deck_health tile under the
standard outage contract; the health grade keeps ignoring it.
Renders met/evaluated with per-check tooltip, mirroring the
consistency tile's three-state null handling. Display-only; the
letter grade ignores it. Key-set pins in the audit tests extended
for the additive consistency_targets key.
…e classifier

contextual_role_targets adjusts ROLE_TARGETS by archetype, commander
role, average mana value, and bracket, per the primer synthesis (Edgar
aggro spec, Gishath resolve-engine deltas, Winota trigger-density
trade, bracket-scaled interaction floors). One documented delta table
is the tuning surface; unknown labels degrade to the flat table.
role_target_report gains a keyword-only context= that is byte-identical
to the old report when absent. infer_commander_role classifies only the
regex-trustworthy roles (cost cheater, trigger multiplier, resolve
engine by mana value) and returns None when unsure.
Four narrow, oracle-signature modifiers from the primer synthesis:
commander_dependence (-8, effect gated on controlling the commander),
tempo_fail (-6, value a turn-cycle late under an aggro plan),
capped_engine (-4, printed once-each-turn limiter), tutor_top_delta
(-5, Vampiric-class top-of-library tutors outside combo shells).
Patterns are deliberately conservative — a miss costs nothing, a false
positive would down-rank a fine card on every audit. Same flag and
validation gate as the rest of FP-015: these refine the ranking prior,
they do not enable it.
nonbo_lint encodes the primer synthesis section-14 table as data-driven
pairwise checks (Skullclamp vs own anthems, Cursed Totem vs own dorks,
forced draw vs Thassa's Oracle, Heartless Summoning vs one-toughness
creatures, mode-exclusive Asceticism/Everlasting Torment, own shroud vs
own targeting, symmetric-effect notes) — the first anti-synergy
detector in the tree. Selectors match by exact name or oracle/type
predicates; a card never pairs with itself; unresolvable cards match
nothing. Wired as the additive-only 'nonbos' deck_health tile with the
standard degrade contract.
…, budget swaps

build_judge_prompt gains a COMMUNITY PRIMER CONTEXT block (clipped,
attention-steering only, identical across A/B orderings) when the
bundled KB covers the deck's commander. The Claude advisor payload
carries community_primer_consensus under the same omit-when-absent
policy as bracket peers, and its system prompt gains the distilled
card-evaluation principles. Budget mode surfaces author-documented
function-preserving swaps for the deck's own commander as paired
add/cut recommendations (supplemental source, same post-processing
as lift picks, fail-quiet).

@LlamaAdam LlamaAdam left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round-3 adversarial review of this PR (maximum-effort pass; every item below was verified against source with file:line and executed reproductions, then cross-examined from the defense side — the cross-examiner's corrected version is what's stated here). Full report with evidence: docs/ollama-analysis/NEGATIVE_MODE_ROUND3.md on LlamaAdam/mtga-advisor#5, §1a and §6.

Owner decisions this PR needs before merge (recorded as R3-D1/R3-D2 in DECISIONS_FOR_REVIEW.md on that PR):

  • Auto-Protect reversal (PR-06, major). This PR removes FP-018.3's primer card-link auto-protection and inverts the test that pinned it (tests/test_adopt.py:239-250). The rationale in the commit is sound engineering — which is exactly why it's a product trade for the owner, not a bug fix; no decision record or review exists for it.
  • Knowledge-base provenance (PR-05 B3). data/primer_kb.json, consistency_targets.py, staples.py, card_score.py, nonbo_lint.py and _advisor_claude.py cite primer_harvest/deckbuilding_heuristics.md §1–§16 and two harvest JSONs that exist in no tree, branch or commit; 35 of the KB's 40 decks have no list or prose anywhere in the repo. Not a code defect — a question of whether an unreproducible data asset ships.
  • Duplicate /api/deck_commander (PR-08, major). #84 adds the same endpoint with a different contract; git merge-tree of the two conflicts in 7 files / 18 hunks, and both declare the same top-level lets in app.js (a SyntaxError for the bundle if both land). Each PR is mergeable_state: clean alone, so GitHub warns nobody.

Fixes for this PR, in priority order:

  1. PR-02 (major) — the import "hardening" now 400s whole pastes master accepted: Maybeboard / Commanders / Tokens headings and Archidekt's default [Commander{top}] tail. Recognise them in normalize_card_line / deck_text_ops rather than rejecting.
  2. PR-03 (major)quoted_win_lines treats any paragraph mentioning "maybeboard" / "Cons:" / "Updates" as a section heading and silences every later paragraph; one in-sentence mention blanks a primer's win lines. Heading heuristic in _primer_tokens must not fire on in-sentence words.
  3. PR-05(A) (major) — the KB budget-swap table emits non-card strings as advisor adds (14/36 rows, e.g. "Kamahl, Heart of Krosa / End-Raze Forerunners", "$200-tier builds"); _safe_ci_lookup skips rather than drops them, so they reach output. Validate rows at load; pin every shipped row as one resolvable card name. Also PR-17 key_cards parses to empty for all 40 profiles (data shape ≠ loader), PR-18 Bruce Banner // The Hulk (gift build) commander key, PR-S2 Ur-Dragon "d10 tokens" (d20).
  4. PR-07 (major)_primer_kb_block puts community-consensus card names into the judge prompt, with no prompt_version on JudgeReport, so the Phase-1 agreement study now pools two prompts with no column to split them — the exact G3 confound the scope doc pre-registered against. Stamp a prompt version; render the block without card names or gate it off by default.
  5. PR-09 (major) — partner decks structurally never reach legal on the dashboard (only the hero commander gets the guarded lookup); fetch the ≤2 command-zone cards the same way.
  6. PR-04 (major) — three of the six FP-019 slices (context-aware roles/quotas, plan-conditional floors, nonbo advisor pre-filter) have zero production callers; either wire them or reword the PR body/CHANGELOG to the future-plans.md STATUS line.
  7. PR-12 / PR-21 — the commander editor and import_deck accept "2 Krenko, Mob Boss" as a card name (executed: the import route writes 1 1 Krenko, Mob Boss and returns 200); the editor also demotes a Protect=-locked card without touching the lock. Reject ^\d+\s names; warn on a locked choice.
  8. PR-01 (minor after cross-exam) — "tests isolate Forge paths" is true for the corpus-reading paths but 13 modules bind VENDOR_FORGE-derived constants at import time, so the autouse fixture cannot reach them; resolve DECK_DIR-style constants at call time and soften CHANGELOG.md:32. PR-13 report normalized_lines; PR-20 main_count changed meaning silently; PR-S1 the Asceticism/Everlasting Torment nonbo "why" states the wrong mechanism; PR-14 five modules now exceed the documented 800-line ceiling; PR-19 the "independent backend/Python/frontend reviews", wheel-build and manual-smoke claims have no artifact (GitHub shows zero reviews) — label or drop.
  9. Mirrors of master findings this PR carries: F-02 adopt.py:165-166 compares card-link names (Front // Back) against front-face .dck names, so every linked DFC is reported "NOT in the list"; W-03 a UTF-8 BOM should be accepted, not 400'd; W-07 the create_app consumer of an unvalidated deck_dir; W-10 PUT line endings.

What held on Linux: the "4,590 passed / 26 Playwright" numbers are substantiated by this PR's own CI; the --run-live gate resists -m/-k/node-id selection; the deck-dir precedence table (8×3 combinations) matches the description; the desktop-lock test isolation is a real fix.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant