| 1 | # Maintainer Playbook — PR triage |
| 2 | |
| 3 | Internal reference for deciding what to merge or close, and how. This is a |
| 4 | maintainer aid, **not an outward promise** — contributor-facing rules live in |
| 5 | [CONTRIBUTING.md](../CONTRIBUTING.md) and the PR template. Precedent PR numbers |
| 6 | are cited so a close can point at a concrete prior decision. |
| 7 | |
| 8 | ## Decision order |
| 9 | |
| 10 | Run a PR through these gates in order. **The first gate it fails is the close |
| 11 | reason** — no need to keep going. |
| 12 | |
| 13 | 1. **Foundational** — does it change an out-of-the-box setting we don't move? |
| 14 | 2. **Capability boundary** — is it outside the "AI-generates-PPT" main line? |
| 15 | 3. **Already-solved** — does the repo already do this, via an existing path or on `main`? |
| 16 | 4. **Root cause & layer** — does it fix the actual cause, at the right layer? |
| 17 | 5. **Evidence & process** — verified against real code, human-reviewed, focused, right entry point? |
| 18 | |
| 19 | Passing all five is the bar for merge. Attitude (last section) applies to every |
| 20 | outcome, merge or close. |
| 21 | |
| 22 | ## The five gates |
| 23 | |
| 24 | ### A. Foundational non-negotiables |
| 25 | |
| 26 | Not merged regardless of implementation quality — these are the project's |
| 27 | factory settings. |
| 28 | |
| 29 | - **License stays MIT** — a deliberate founding choice, not changed midway (#203). |
| 30 | - **`pip + requirements.txt` is the only official install path** — no `uv`/`poetry` as a required dependency; multiple dependency sources are a lifetime sync cost that lands on the maintainer (#92, #116). |
| 31 | - **No CI / test frameworks / lint infra**; `tests/` and `test_*.py` are disallowed by `docs/rules/code-style.md` §11 (#200, #203). |
| 32 | - **No fixed numeric quotas** (`max_cards`/`max_bullets`/`max_table_rows` …) — density is governed by narrative rhythm and one primary focus per page (#203). |
| 33 | - **No `_CN` (or other translated) copies of governance docs** — they drift without a sync owner (#179). |
| 34 | - **AI generates a deck; it does not template-fill one** — DrawingML component reuse / native-diagram is outside the current direction (#153–#168). |
| 35 | |
| 36 | ### B. Capability boundary |
| 37 | |
| 38 | The project is a tool around the "AI generates PPT" main line, not an |
| 39 | everything-repo. "We know our limits" is itself the rule. |
| 40 | |
| 41 | - **Post-processing is for compatibility, not quality smoothing** — we fix things that are *broken/unusable if not done* (AI-image size/format/alpha). Loudness normalization, kerning polish, etc. are quality smoothing and stay out (#194). |
| 42 | - **If a model/service doesn't work, switch it — don't make the project adapt to it.** A weak agent/provider is a capability gap on their side (#194; also the prompt-change rule). |
| 43 | - **Secondary opt-in features don't get to add heavy deps or change global defaults** — e.g. a narration sub-issue may not push tens-of-MB ffmpeg into core `requirements.txt` default-on (#194). |
| 44 | - **Fidelity yes, authoring deferred** — preserving existing links/structure is in scope; inventing new links/structure on the generation side waits until the core is stable (#155). |
| 45 | - **Personal / vertical preferences belong in the contributor's own copy** (`CLAUDE.md` / fork), so the shared skill stays general-purpose (#177). |
| 46 | |
| 47 | ### C. Already-solved / redundant |
| 48 | |
| 49 | Verify current behavior before accepting a "fix" or "feature". |
| 50 | |
| 51 | - **The capability already exists, docs were just unclear** — manual image gen was already covered by `Needs-Manual` (#88). |
| 52 | - **Already supported via an existing path** — Agnes runs under `IMAGE_BACKEND=openai`; a separate backend duplicates the compatibility layer (#202). |
| 53 | - **`main` already solves it more completely** — text-box width is handled at run level (#200). |
| 54 | - **Don't duplicate an existing abstraction/path** (#202, #88). |
| 55 | |
| 56 | ### D. Root cause & correct layer |
| 57 | |
| 58 | - **Fix the root cause; don't paper over an attribute/state nobody consumes** — removing an unconsumed `width`/`height` requirement beat backfilling it (#221). |
| 59 | - **Checks/validation should guard only what downstream actually consumes** (#221). |
| 60 | - **Respect abstraction layers and the single confirmation point** — image strategy is a Step 4 (Eight Confirmations) decision; Step 5 is execution, not a place to re-decide (#88). |
| 61 | - **Take the valid signal, land it our way** — fold the real gain into the existing path instead of swapping an engine or adding a dependency (#207). |
| 62 | |
| 63 | ### E. Evidence & process |
| 64 | |
| 65 | Mirrors CONTRIBUTING; these are close-on-sight. |
| 66 | |
| 67 | - Every factual claim must be verified against this repo's actual code — AI-invented problem narratives are closed regardless of diff quality. |
| 68 | - Purely AI-generated, personally-unreviewed PRs are closed unmerged. |
| 69 | - **The PR template's three confirmation boxes: any one left unchecked = closed without review.** |
| 70 | - One PR, one thing; focused bug-fix PRs are prioritized (#206). |
| 71 | - **Prompt/instruction changes** (`SKILL.md`, `references/*.md`, `workflows/*.md`) require a prior agreed issue before a PR. |
| 72 | |
| 73 | ## How I close (applies to every outcome) |
| 74 | |
| 75 | 1. **Thank sincerely, and separate "not merging" from "your work is poor."** Name the gate (boundary / direction / already-solved), not the person. |
| 76 | 2. **Take and credit the valid signal** — `Co-authored-by`, land it in our structure (#155, #207). |
| 77 | 3. **When it's timing/direction, leave the door open** — "we can reopen and restart from here" (#168). |
| 78 | 4. **Redirect to the right entry point** — open an issue first, use existing config, keep it in your fork (#177, #202, #206). |
| 79 | |
| 80 | ## Canned closes |
| 81 | |
| 82 | Reusable openers — adapt specifics, keep the tone above. Fill `<…>`. |
| 83 | |
| 84 | **Unchecked confirmation box** |
| 85 | > Thanks for the PR. Closing per CONTRIBUTING — the template's three confirmation boxes must all be checked (personal review + claim verification, and the code-only/prompt-issue line), and `<box>` is unchecked. Once you've done `<X>` and can confirm it, feel free to reopen. |
| 86 | |
| 87 | **Prompt change without a prior issue** |
| 88 | > Thanks. This edits prompt/instruction text (`<file>`), which per CONTRIBUTING needs a prior agreed issue before a PR — these files steer AI behavior deck-wide and sit near a fixed token budget, and restating a rule the docs already state rarely fixes a non-compliant agent. Please open an issue describing the failure so we can agree the direction first. |
| 89 | |
| 90 | **Already supported via existing path** |
| 91 | > Thanks for the contribution. Closing because `<capability>` is already available through `<existing path>` — use `<config/command>`. A separate `<thing>` duplicates the existing path and runs against the current design in `<doc>`. If `<provider/case>` later needs behavior the existing path can't express, we can revisit with a focused change. |
| 92 | |
| 93 | **Already solved on main** |
| 94 | > Thanks — the root-cause analysis is correct and matched ours. Closing because `main` already handles this, more completely, via `<mechanism>` in `<file>`. It just landed first through a different structure. Really appreciate the issue and the PR. |
| 95 | |
| 96 | **Out of scope / capability boundary** |
| 97 | > Thanks, and for the careful write-up 🙏 After weighing it I'm not taking this direction — it's a capability-boundary call, not an implementation-quality one. `<Reason: quality-smoothing vs compatibility / disproportionate dependency cost / belongs on the provider side>`. If `<X>` blocks you, the more reliable path is `<user-side one-liner / switch provider>` rather than welding it into the default pipeline. |
| 98 | |
| 99 | **Foundational setting won't change** |
| 100 | > Thanks for the PR, but this isn't something I'll merge: `<setting>` is a foundational choice from the start and won't change midway. `<one line why>`. Closing that outright. |
| 101 | |
| 102 | **Personal / vertical → own copy** |
| 103 | > Thanks for your interest! This looks meant for your own workspace/fork — the best home is your project `CLAUDE.md` or a personal fork, so you get exactly the behavior you want without changing the shared skill (which stays general-purpose). Closing here, but please don't take it as discouragement — an issue is always a great way to float an idea first. |
| 104 | |
| 105 | **Tiny fix → issue** |
| 106 | > Thanks! For a fix this small, an issue is usually faster for me to apply directly than a PR round-trip. Closing this, but the report is genuinely useful — mind opening an issue with `<detail>`? |
| 107 | |
| 108 | **Not a hard no (timing/direction)** |
| 109 | > Thanks for all the work here 🙏 This sits outside the project's current focus rather than being a problem with the implementation. To keep the PR list manageable I'll close it as not-planned for now — not a hard no; when we revisit this direction we can reopen and restart from here. |
| 110 |