| 1 | # Codex review brief — the Slides design, revision 1 |
| 2 | |
| 3 | You are GPT-6, reading as an independent second reader. Claude drafted this |
| 4 | spec and built three slices against it; nobody else has read it. §12 of the |
| 5 | spec says the review pass waits until a reviewer can have a running |
| 6 | instance instead of a document. That is now true, so this is that pass. |
| 7 | |
| 8 | Answer as findings, not as prose. Number them. For each: the section or |
| 9 | file, what is wrong or unsound, why it matters, and what you would do |
| 10 | instead. Mark severity P1 (a security hole, data loss, or a ruling that |
| 11 | cannot hold), P2 (must fix before the next slice), P3 (worth doing). |
| 12 | Cite code by path and line when you make a claim about the code. If you |
| 13 | cannot verify something from the repository, say so rather than assuming. |
| 14 | |
| 15 | ## What to read |
| 16 | |
| 17 | Repository: `oficina/slides`, this worktree, at commit `6c757a4` (branch |
| 18 | `slice-3-export`). Three slices are built and deployed. |
| 19 | |
| 20 | - The spec: `docs/superpowers/specs/2026-09-07-slides-design.md` (revision 1, |
| 21 | thirteen rulings in §1). §4.2 and §4.3 are being amended in a commit that |
| 22 | is in flight as you read; judge the design, not those two paragraphs' |
| 23 | wording. |
| 24 | - The plans, which say what each slice meant to build: |
| 25 | `docs/superpowers/plans/2026-09-07-slice-1-live.md`, |
| 26 | `2026-09-07-slice-2-editor.md`, `2026-09-08-slice-3-export.md`. |
| 27 | - `AGENTS.md` — the seams, and the traps a new session must know. |
| 28 | - The suite doctrine this app consumes by ruling number: |
| 29 | `../../../suite/docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md` |
| 30 | (revision 8). The spec's §0.2 lists which rulings it leans on. |
| 31 | |
| 32 | The code, if you want to check a claim: `internal/slides/` (the app; |
| 33 | `auth.go`, `decks.go`, `viewer.go`, `share.go`, `presentation.go`, |
| 34 | `live.go`, `editor.go`, `assets.go`, `thumbs.go`, `export.go`, `api.go`, |
| 35 | `agents.go`), `internal/slides/{core,pdfcore,layout,svg,fonts,render,deck,live,present}/`, |
| 36 | `core/` and `core/pdf/` (the two Rust crates), `fixtures/` (the browser |
| 37 | half and the op vocabulary that must mirror `core/src/ops.rs`). |
| 38 | |
| 39 | ## The questions I most want a cold reader on |
| 40 | |
| 41 | 1. **The one rule** (§0.1): "a slide is one block tree, rendered many |
| 42 | ways". Three renderers now exist — `render.Slide` (HTML), `layout` + |
| 43 | `svg` (pictures and PDF), and ProseMirror binding into the server's |
| 44 | HTML in the browser. Does the rule still hold, or has it been broken in |
| 45 | practice while the words stayed? |
| 46 | 2. **The hybrid model** (§2.3, ruling 1): flow templates and a canvas |
| 47 | layout in one block tree, with a conversion between them |
| 48 | (`core/src/layout.rs`). Is the conversion sound, or does it lose work in |
| 49 | a way a person will hit? What happens to a deck that is switched back |
| 50 | and forth? |
| 51 | 3. **The phone layer** (§2.5, ruling 2): a `phone` map per block, written |
| 52 | by the editor's Phone view, read by the renderer, never read by print. |
| 53 | Is "the desktop layout is always the base; the phone layer only adds" |
| 54 | a rule that survives contact with real decks? |
| 55 | 4. **Trust in the write path.** Any member may POST arbitrary Yjs bytes |
| 56 | (`live.go`); the core applies them, the app parses the resulting view |
| 57 | and refuses a batch that would leave the deck unrenderable. Is that gate |
| 58 | in the right place, and is there a shape it misses? |
| 59 | 5. **Capabilities** (§5, doctrine ruling 15): a share link is a hashed |
| 60 | read-only token; a presentation session's sqid lets anyone view, its |
| 61 | remote token lets a holder drive, its pairing code lets a phone find it. |
| 62 | Four capability kinds across `share.go`, `presentation.go`, `thumbs.go` |
| 63 | and `assets.go`. Is any of them stronger than it should be, or |
| 64 | confusable with another? |
| 65 | 6. **Agents as principals** (§7, ruling 7): tokens minted in-app by a |
| 66 | person, `agent:<id>` recorded as the actor, the API mounted outside |
| 67 | CSRF because a bearer caller sends no browser headers. Sound? |
| 68 | 7. **The export path** (§4.2, §4.3, ruling 13): a Go layout engine |
| 69 | measuring with embedded Liberation faces, an SVG per slide, and |
| 70 | `svg2pdf` in a wasm module under wazero. With no object store bound the |
| 71 | finished PDF is held in memory for an hour, capped. Where does this |
| 72 | break first? |
| 73 | 8. **What the spec does not say** that a second reader would expect a |
| 74 | presentation editor's design to settle before slice 4. |
| 75 | |
| 76 | ## What is deliberately not built |
| 77 | |
| 78 | Spec §9 lists it with triggers: sign-in through the suite home (it is a |
| 79 | skeleton), charts from Sheets, video and embeds, an MCP adapter, aspect |
| 80 | ratios other than 16:9, comments, version history. Do not report those as |
| 81 | gaps; do report it if one of them is load-bearing for something the spec |
| 82 | claims today. |
| 83 | |
| 84 | ## Conventions of this house, so you can judge against them |
| 85 | |
| 86 | Every user-visible string is a catalog key. Every screen but the editor |
| 87 | works with scripts off. WCAG 2.2 AA with no exemptions. One static Go |
| 88 | binary per team on the CARLOS platform, `CGO_ENABLED=0`, SQLite, no |
| 89 | Chromium anywhere. `fixtures/` and the Rust core are two implementations |
| 90 | of one op vocabulary, held together by committed vectors. The gate |
| 91 | (`.amadan/ci.d/10-gate`) rebuilds both wasm modules and the browser bundle |
| 92 | and diffs them against what is committed. |
| 93 | |
amadan