arthur/xtablo-source
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions 5

docs(01): cross-AI review for phase 1 (codex)

Browse source
This commit is contained in:
Arthur Belleville 2026-05-14 17:23:11 +02:00
parent ad3a3d42ef
commit a54a10a7fd
No known key found for this signature in database
1 changed files with 81 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

81
.planning/phases/01-foundation/01-REVIEWS.md Normal file
View file

@ -0,0 +1,81 @@
---
phase: 1
reviewers: [codex]
reviewed_at: 2026-05-14T15:23:07Z
plans_reviewed: [01-01-PLAN.md, 01-02-PLAN.md, 01-03-PLAN.md, 01-04-PLAN.md]
skipped: [claude (self), gemini (not installed), coderabbit (not installed), opencode (not installed), qwen (not installed), cursor (not installed), ollama (not running), lm_studio (not running), llama_cpp (not running)]
---
# Cross-AI Plan Review — Phase 1: Foundation
## Codex Review
## Summary
The plans are unusually thorough and trace requirements well, but they are too brittle in a few places. The biggest risks are not architectural; they are execution-contract issues: commands that may not work as written, tests that are intentionally red but may block normal `go test ./...`, and a few contradictions between “vendored/static/no CDN/no generated files” policies and the proposed bootstrap flow. Overall, the phase goal is achievable, but the plans need tightening before execution.
## Strengths
- Clear phase boundary: Foundation stays focused on scaffold, health checks, templ/HTMX, local Postgres, migrations, logging, and README.
- Good traceability from FOUND-01..05 to concrete files, commands, tests, and checkpoints.
- Strong dependency sequencing: scaffold → UI/tests → green implementation → README.
- Good operational basics: request IDs, structured logs, graceful shutdown, timeouts, DB health behavior.
- Strong UI discipline for a small phase: tokens, components, CSS ownership, and no raw button styling in page templates.
- Good local-dev validation, including browser HTMX checks and README onboarding walkthrough.
## Concerns
- **HIGH: `go mod tidy` in Plan 01 may remove all pinned runtime deps.** Since no Go code imports chi/templ/pgx/goose/uuid yet, `go mod tidy` can drop those `require` lines. The acceptance checks may fail immediately.
- **HIGH: Tailwind standalone download URL is likely fragile.** `tailwindcss-$(uname -s | tr A-Z a-z)-$(uname -m)` often does not match GitHub asset naming, especially macOS `darwin` vs `macos` and `x86_64` vs `x64`.
- **HIGH: Intentional RED tests can break later scaffold verification if not isolated.** Plan 02 says `go test ./internal/web` must fail, but also introduces `pool_test.go` referencing `db.NewPool` before it exists. Any accidental `go test ./...` after Plan 02 will fail beyond the intended web RED gate.
- **MEDIUM: “Vendored” HTMX is gitignored.** The plan says HTMX is vendored, but `.gitignore` excludes `static/htmx.min.js`. That makes it bootstrap-generated, not repo-vendored. This is workable locally, but the wording and later deployment assumptions should be consistent.
- **MEDIUM: CDN policy is inconsistent.** Success criteria say “No CDN references in any committed file,” while the justfile intentionally contains an `unpkg.com` bootstrap URL. Later verification allows this exception. Make the exception explicit everywhere.
- **MEDIUM: Plan 01 `just generate` will fail until Plan 02 creates UI CSS and Plan 03 creates templates/cmd.** This is acceptable if documented, but the justfile exists before its recipes are fully runnable.
- **MEDIUM: UI component CSS may not pass Tailwind v4 processing as written.** Plain nested CSS like `&:hover` is only safe if the Tailwind pipeline supports nesting in that context. Prefer non-nested `.ui-button-solid-default-md:hover`.
- **MEDIUM: `templ Card(attrs templ.Attributes)` with children needs exact templ syntax.** The plan’s interface is conceptually right, but templ child syntax should be verified against current templ rules before locking tests around it.
- **LOW: Goose no-op migration with `SELECT 1` is fine but aesthetically odd.** It exercises goose, but creates no domain object. That is acceptable for Phase 1.
- **LOW: README asks for `just clean`, but no `clean` recipe exists.** It is listed only in fallback Method B, but it violates the “no aspirational commands” rule unless added or removed.
## Suggestions
- Fix Plan 01 dependency pinning: either skip `go mod tidy` until code imports the deps, add actual imports in early files, or accept that deps land in Plan 03 rather than Plan 01.
- Replace Tailwind URL logic with explicit OS/arch mapping in shell, covering `darwin → macos`, `x86_64 → x64`, `aarch64/arm64 → arm64`.
- Change the CDN rule to: “No runtime CDN references; bootstrap-time download URLs are allowed only in `justfile`.”
- Decide whether HTMX is committed or generated. If generated, call it “bootstrapped local asset,” not “vendored,” and ensure deploy/build docs run bootstrap.
- Make Plan 02 verification only run package-specific checks that tolerate the intentional RED state. Avoid any command that implies `go test ./...` should pass before Plan 03.
- Use non-nested CSS selectors in component CSS for maximum compatibility.
- Remove `just clean` from README Method B or add a real `clean` recipe.
- Add one explicit “Plan 01 incomplete recipe caveat”: `generate`, `test`, `build`, and `dev` are listed early but only become runnable after Plans 02/03.
## Risk Assessment
**Overall risk: MEDIUM.**
The architecture is low-risk and appropriate for the phase, but execution risk is medium because several command contracts are likely to fail exactly where the plan expects mechanical reliability: Go module tidying, Tailwind binary download, generated asset policy, and intentional red-test handling. Tightening those will make the plans strong enough to execute.
---
## Consensus Summary
Only one external reviewer (Codex) was available — the host AI is Claude Code (skipped for independence), and no other CLIs or local model servers were installed/running. A true consensus across multiple AI systems is not possible from this run; treat the Codex review below as a single independent perspective rather than convergent feedback.
### Agreed Strengths (Codex only)
- Tight phase boundary — Foundation stays scoped to scaffold, health checks, templ/HTMX, Postgres + migrations, logging, README.
- Clear FOUND-01..05 traceability into concrete files, commands, tests, and checkpoints.
- Strong sequencing: scaffold → UI/tests (RED) → GREEN implementation → README.
- Good operational basics: request IDs, structured logs, graceful shutdown, DB-health semantics on `/healthz`.
### Agreed Concerns (Codex only — HIGH severity)
1. **`go mod tidy` in Plan 01-01 may strip pinned runtime deps** (chi, templ, pgx, goose, uuid) because no Go file imports them yet.
2. **Tailwind standalone download URL is fragile** — `uname -s` / `uname -m` outputs don't match GitHub release asset naming (`darwin` vs `macos`, `x86_64` vs `x64`).
3. **Intentional RED tests in Plan 01-02 can leak** — a stray `go test ./...` will fail beyond the intended web RED gate (e.g., `pool_test.go` referencing `db.NewPool` before it exists).
### Divergent Views
Not applicable — only one reviewer.
### Recommended Next Step
Re-run `/gsd:review --phase 1` after installing at least one additional independent CLI (e.g., `gemini`, `codex` already present, plus `coderabbit` for diff-based review or `opencode`/`qwen`/`cursor`) so concerns can be cross-validated. In the meantime, the three Codex HIGH-severity concerns above are concrete and actionable — feed them into `/gsd-plan-phase 1 --reviews`.