Files
wow20/_bmad-output/planning-artifacts/architecture/architecture-wow20-2026-08-14/reviews/review-rubric.md
T
2026-08-14 19:05:39 +02:00

163 lines
18 KiB
Markdown

# Rubric Review — Wiki of Wikis Architecture Spine
Review type: good-spine checklist walker
Reviewed artifact: `_bmad-output/planning-artifacts/architecture/architecture-wow20-2026-08-14/ARCHITECTURE-SPINE.md`
Row-up PRD: `_bmad-output/planning-artifacts/prds/prd-wow20-2026-08-14/prd.md`
Reviewed against: good-spine checklist; capabilities FR-1..FR-16, NFR-1..NFR-7 of the PRD
Review date: 2026-08-14
---
## Gate verdict
**CONDITIONAL PASS** — the spine is strong and internally coherent, but it fails to fully close the multi-producer coordination concern (AD-17 is a convention, not an enforced rule), and its Operational Envelope is too thin on security/access and on the operational concurrency and environment envelope. One access A uses the word "Simulation" in a place where an enforced rule is needed, and the "no security" reality needs an explicit, honest decision.
---
## Checklist results
| # | Criterion | Result | Note |
|---|---|---|---|
| 1 | Fixes the real divergence points for the level below; misses none | PARTIAL | AD-17 identifies the real LWW risk, but the fix is a convention, not an enforcement mechanism (see Finding C-1, C-2). |
| 2 | Every AD's Rule is enforceable and prevents its stated divergence | PARTIAL | Rules are mostly enforceable via admission-control tests and fitness tests, but AD-4's Rule 2 ("material claims must remain transitively grounded") is not mechanically checkable without an auditor that follows `sources` transitively. This is the strongest "divergence prevention" gap. |
| 3 | Nothing under Deferred lets two units diverge | PASS | D-1 (source acquisition adapters), D-2 (retrieval), D-3, D-4, D-5..D-8 — either explicitly single-producer or consumer-side; none contradict an AD. Note D-7 (human review workflow) is a workflow choice, not a data-model divergence. |
| 4 | Named tech is verified-current | PASS | OKF 0.2 confirmed current via direct fetch; Markdown, Git, ripgrep all current/standard. |
| 5 | Covers the spec's capabilities (FR-1..16, NFR-1..7) | PASS | Every FR and NFR has at least one governing AD (see coverage table). |
| 6 | Every dimension the altitude owns is decided, deferred, or an open question | FAIL | The operational/environmental envelope is incomplete: security/access (team sharing) is silent; environments/deployment are effectively never decided; operational concurrency handling of the lease mechanism on a shared workspace is undefined. |
---
## Detailed findings (tiered)
### Critical
#### C-1 — AD-17's "Lease" is a convention, not an enforced rule; Last-Writer-Wins is not actually fixed
**Where:** AD-17, bullet 3 (line 445-451) and bullet 4 (line 453)
AD-17's stated prevent is exactly the Last-Writer-Wins problem the rubric asked about: two producers (Claude, Codex, or the human) mutating the same concept path simultaneously. The cited mechanism is:
- Each producer works in its own branch/worktree and resolves conflicts at Git level (rebase/merge) itself (rule bullet 1).
- The compiler only consumes committed content (rule bullet 2).
- "He'd'den applies: acquire lease, mutate only within leased area, release lease; conflict resolution by productive value logged in `log.md`" (rule bullet 3-4).
The problem:
1. The word "Lease" is a word, not a mechanism. There is no definition of how a lease is acquired, how it is held exclusive, how it is released, or what happens when the acquiring producer dies holding the lease. On a plain filesystem+Git workspace there is no enforcement primitive for exclusivity; two producers contended on the same path in two worktrees will both commit, and Git merge will either produce a conflict (both edited the same line — then the productive-value principle in `log.md` is an advisory that no process enforces) or a clean merge by LWW semantics if they edited different lines of the same file.
2. Rule bullet 1 says conflict resolution happens **at Git level by the producer itself** — that reintroduces exactly the LWW/second-writer-wins the AD claims to prevent: there is nothing that prevents producer B from silently overwriting producer A's concept path with its own diff, because "productive check via log.md" is a human-opinion algorithm, not a checkable invariant.
3. The compiler itself (mutator) is invoked by the adapter (agent). The AD says convergence is "orchestrated by the executing adapter" — but with no executable lease protocol, nothing stops two adapters from running concurrently against the same repo. The result is exactly the LWW silent overwrite the AD says it prevents.
Grades:
- **Enforceability:** No. A reviewer following the Rule mechanically cannot verify compliance at commit time without a lease table that is itself a derived artifact contradicting AD-1's "derived artifacts are not source of truth", and even then nothing enforces it.
- **Divergence prevented:** Not fully. This is the single most important gap in the whole spine.
Recommended fix (before implementation):
- Replace the prose lease with a minimal, mechanically checkable protocol: e.g., a `locks/<concept-path>.lease` convention (a file whose presence means "leased", whose content is producer + timestamp + expected completion, whose existence is checked by the compiler entrypoint and by every commit hook), plus a hard rule that a commit that touches a path where the lease is not held by the committing producer is rejected by a pre-commit hook. If "no runtime" is sacred, the lease must be a file, not a memory object; a git commit hook reading a small text file is a normal file operation, consistent with the no-runtime philosophy.
- Alternatively, explicitly downgrade AD-17 to a single-writer-acknowledged model ("the compiler assumes only one producer at a time; if two producers contend, the human resolves at Git level and the result is LWW for that path") and say so. That would at least be honest and enforceable; the current text implies more safety than the mechanism provides.
#### C-2 — Two producers, two worktrees, same repo: each branch holds the whole bundle; the term "Won't run concurrently" is NEVER stated
**Where:** AD-17 rule bullet 1; also AD-6, AD-3
There is no statement anywhere that the compiler is single-instance. AD-17's own scenario ("Claude, Codex, or the human") is three possible producers; nothing in the spine limits the number of simultaneous compilation runs that may feed the same bundle. If the intent is single-writer serialized runs, that intent is nowhere made explicit, and the reader cannot distinguish "concurrent adapters are coordinated by the lease", which is claimed, from "concurrent adapters are inherently excluded", which is not claimed and would be desirable to state.
Fix: add an explicit statement in AD-17 (or an AD-17a): "At most one compilation run may be active against the shared workspace at a time; runs are serialized by file-lock or by the existence of a `locks/`-style lease." If this is a hard design constraint, the spine must say so; otherwise the claim that LWW is prevented overstates a mechanism that does not exist.
### High
#### H-1 — Operational Envelope is thin: security/access for the team case is silent; environments never decided
**Where:** "Operational Envelope" section (lines 564-590); PRD A-1 (§ 2.1 assumption: "primär ein persönliches bzw. teaminternes Developer Tool")
The envelope covers only Local-first execution, Failure behavior, Portability. The checklist specifically asks about failure behavior, security/access, environments:
- **Security / access (team case):** The PRD's own assumption A-1 says the product is a "persönliches bzw. teaminternes Developer Tool" — the primary user is "ein technisch versierter Wissensarbeiter bzw. Softwareentwickler." If team-internal, then "who writes" (push permissions to `raw/` and `wiki/`; who may produce leases; who may mark `verified`) is a real divergence point the architecture owns. The spine is silent on all of it. Git-level collaboration (a shared repo) is the natural answer, but the spine never states that the team access story is "Git repository permissions" vs. "anyone with filesystem access", nor what the boundary is for `raw/` (immutable evidence) vs `wiki/` (mutable) from a writes standpoint. A team member editing `wiki/` directly is allowed per AD-15/Q-2, but nothing governs concurrent human-vs-compiler writes beyond AD-17 (which is underexplained as a mechanism — see C-1).
- **Environments / deployment / CI:** The spine says nothing at system level about `main` vs worktrees, about a shared remote, about where compiled bundles are published, about promotion (e.g., lease branch → merge → main). Since the spine binds Git, environment decisions (which branch is canonical read-surface, whether compilation runs on `main` or on a branch that is later merged) are a real divergence point for multiple consumers reading "the wiki" — two consumers polling different branches see different knowledge. This should be explicitly decided or explicitly deferred (it is neither).
Impact: this is dimension (6) in the rubric — a whole dimension left silent. For a single-user local tool it would be acceptable; for a team-shared Git workspace it is a finding.
#### H-2 — AD-4 Rule 2 (transitive grounding) is not mechanically enforceable
**Where:** AD-4 Lines 112-140; fitness test FT-5, FT-4
The Rule says "materielle fachliche Aussagen müssen auf nachvollziehbare Sources zurückgeführt werden" and "Bei einer Synthese aus mehreren Concepts wird relevante Source-Provenienz in das resultierende Concept übernommen." FT-5 ("its material source basis can be traced to actual Source Material") is the test, but nothing specifies a mechanical check that a synthesized concept actually carries provenance that reaches a `raw/` file. As written it is a semantic preference, not a verifiable invariant: a compiler could produce a concept with `sources` referencing another concept's ID only, and no admission check would fail.
Fix: define the check as part of the Compiler contract (schema/wiki-compiler.md): for every generated concept, `sources` entries must resolve (transitively, bounded depth) to at least one source under `raw/` (or a documented external immutable reference); a concept whose entire `sources` chain terminates in other concepts fails validation and the run fails (or the concept is downgraded). This is checkable with grep/traversal — consistent with the "no runtime/vector infra" constraint.
### Medium
#### M-1 — Consistency Conventions table "Erstellt von" / "Errors" entry is gray
**Where:** "Consistency Conventions" table, row "Errors" (line 474): "kein erfolgreicher Run bei ungültigem Bundle"
In AD-6 (Overlap with AD-6) — acceptable summary, but the table generalizes to "invalid bundle" with no definition of what validates it and who runs validation. Combined with H-2, the table promises an acceptance gate that the document does not give a mechanism for. Minor: the row belongs to the "no-proprietary-format" axis and would be more useful as "validation = OKF conformance + AD-4 grounding, implemented as committed checks in schema/". Recommend rewording to name the artifact that performs it.
#### M-2 — AD-16 "classifies new information" is conceptual only; "Relevance determination" reference leaks implementation
**Where:** AD-16 lines 419-434; AD-17 "Wichtig — funktionaler Grund" (line 455)
AD-16 says the classification "ist konzeptionell; sie schreibt keine konkrete interne Datenstruktur vor" — good and consistent with AD-6. But the "Wichtig" note in AD-17 (line 455) makes a strong causal claim: "Relevanzbestimmung (PRD OQ-3...) ist begrenzt: Sie muss mit textuellen, deterministischen Mitteln (grep/ripgrep, Markdown-Traversal, Link-Following) umgesetzt werden, nicht mit Embedding- oder Vector-Infrastruktur." This is correct and consistent with AD-13, but it belongs in AD-5/AD-13 (relevance determination is a compilation-input concern, not producer mechanism) rather than AD-17, where a reader of AD-17 is unlikely to look for it. The PRD explicitly hands this decision to the architecture (PRD § 12), and the spine does answer it — but in the wrong section. Reorganize: relevance determination deserves its own AD-18 (or a bullet in AD-5) so the "how does the compiler find relevant existing concepts" answer is not buried in the leasing AD.
### Low
#### L-1 — AD-11 label "AD-11 — Agent runtime provides reasoning; Wiki of Wikis provides protocol" is a naming shift from "protocol" semantics
**Where:** AD-11
The title implies WoW "provides protocol", but the SVG/diagram and rule text describe WoW as instructions ("kanonische Compiler-Regeln"), not a wire protocol. If it is an *agent-instruction contract* (which AD-10 says, correctly), the title should say "Compilation Contract" rather than "protocol", to avoid implying a wire/serialization protocol where none is designed. Cosmetic, but affects how a downstream implementer reads the boundary.
#### L-2 — Missing explicit statement tying D-7 (Human review workflow) to a resolved OQ
**Where:** D-7 (line 707-713) with PRD OQ-2 (§ 10), AD-15
PRD OQ-2 ("Verification Workflow") is answered in the PRD's own NOTE FOR PM ("v1-Default ist maschinell erzeugt und ungeprüft") and mirrored in AD-15. D-7 correctly defers the workflow itself. This is coherent, but the spine never states which PRD OQ each AD resolves — adding a small column ("Resolves/answers") mapping the six PRD OQs (and § 12's two handed-off questions, which are answered in AD-17's note and hence belong elsewhere per M-2) would let a reviewer see at a glance that no PRD open question is left dangling. It also would have made M-2 visible earlier.
---
## Coverage verification (checklist item 5)
Every FR and NFR of the PRD has at least one governing AD; the spine's Capability → Architecture Map (lines 518-537) is accurate and complete. Cross-check against the PRD:
| Capability | Spine's governing AD(s) | Also relevant |
|---|---|---|
| FR-1 Source intake | AD-2, AD-3, AD-12 | AD-17 rule 2 (immaterial during run) |
| FR-2 Source/Knowledge separation | AD-2 | |
| FR-3 Provenance | AD-4 | AD-16 (preserve provenance in contradictions) |
| FR-4 Existing knowledge processing | AD-5 | |
| FR-5 Concept creation | AD-5, AD-7 | AD-1 |
| FR-6 Concept update | AD-5 | AD-4 |
| FR-7 Cross-source synthesis | AD-4, AD-5 | |
| FR-8 Contradictions | AD-16 | |
| FR-9 OKF compliance | AD-1 | AD-15 (trust metadata) |
| FR-10 Relationships | AD-8 | |
| FR-11 Progressive discovery | AD-9 | |
| FR-12 Incremental evolution | AD-5 | |
| FR-13 Human curation | AD-15 | |
| FR-14 Change history | AD-14 | |
| FR-15 Tool-independent access | AD-1, AD-8, AD-9 | AD-13 |
| FR-16 Consumer independence | AD-10, AD-13 | |
| NFR-1 Portability | AD-1 | Operational Envelope (Portability) |
| NFR-2 Human readability | AD-15 | Consistency table |
| NFR-3 Agent readability | AD-1, AD-9 | AD-13 |
| NFR-4 Version-control friendliness | AD-14 | |
| NFR-5 No mandatory runtime | AD-1, AD-11 | Stack table |
| NFR-6 Vendor independence | AD-1, AD-10, AD-11 | Stack table |
| NFR-7 Graceful partial knowledge | AD-4, AD-16 | |
No gaps. All 16 FR + 7 NFR covered.
## Tech currency (checklist item 4)
Verified via direct fetch of `https://github.com/GoogleCloudPlatform/knowledge-catalog/blob/main/okf/SPEC.md`: the current OKF version is **0.2** (matching the PRD's § 13 reference). Key OKF-0.2 facts used by the spine are accurate: `type` is the only required frontmatter key; `timestamp` was superseded by `generated.at` (the spine never uses `timestamp` — good); `sources` supersedes the body-level `# Citations` list (the spine uses `sources` — consistent); `verified` absent means unverified; `status` default `stable`; `okf_version` only allowed in bundle-root `index.md`; `references/` is a naming convention, not a requirement; conformance rule that consumers MUST NOT reject bundles for missing optional fields / unknown keys / broken links / missing `index.md` — all consistent with AD-1/AD-15/AD-16 phrasing.
Note: OKF 0.2 gotcha — the spec says `index.md` and `log.md` are RESERVED filenames that "MUST NOT be used for concept documents". The spine uses `wiki/log.md` (line 490) for disagreements/history (AD-16 line 433). That is consistent with the reserved-name contract (a `log.md` that is not a concept is fine per spec), but the spine never states this distinction; a naive implementer could treat `log.md` as a concept and fail OKF conformance. Add one sentence to AD-16/AD-14: "`log.md` and `index.md` are reserved non-concept filenames per OKF; they must never carry a `type`." Low/severe depending on validator strictness.
## Placeholder / hygiene check (added at request)
- No duplicate AD-IDs; no `TODO`/`FIXME`/`TBD`/placeholder text found (grep -i). AD-1..AD-17 each has a unique, descriptive title.
- Every AD carries Binds/Prevents/Rule (AD-11/AD-12 "Binds" uses 'MVP'-style target but structure present). All ADs have a matching `[ADOPTED]` or `[ASSUMPTION]` tag (8 assumptions, 9 adopted). Challenge: AD-4, AD-6, AD-11 are assumptions but their Rules read as normative ("Ein Compilation Run muss …") — the tag versus the certainty of the Rule body is inconsistent; potential reader confusion about whether AD-4's grounding is assumed or required. Medium-touch: retag to `[ADOPTED]` if the Rule is truly binding, or soften the Rule text if it's exploratory.
- Status field says "draft" (frontmatter line 8) while content reads as adopted; fine for a spine artifact in progress, but flag for the level below that the spine is not final.
---
## Summary of stops to fix before the next level
1. **AD-17 mechanism** (C-1): turn lease/conflict-convention into a mechanically checkable file-based protocol (lease file + commit hook) or explicitly downgrade to single-writer serialization. As written, LWW is not actually prevented.
2. **Concurrency statement** (C-2): state explicitly "at most one active compilation run / serialization by file-lock" or say concurrent runs are excluded; don't imply safety that doesn't exist.
3. **Operational Envelope** (H-1): add a short "Security & Access" subsection for the team-internal case (who writes `raw/` vs `wiki/`, who may mark verified, whether access = Git repo permissions) and state the environment/deployment stance (canonical branch, promotion of lease-branch → main, whether a shared remote exists) as either decided or explicitly deferred.
4. **AD-4 enforceability** (H-2): specify the transitive-grounding admission check in the compiler contract (schema/wiki-compiler.md) so FT-5 is real.
5. **Relevance-determination placement** (M-2): give the "how does the compiler find relevant concepts with text-only means" answer its own home (AD-5/AD-18), not inside AD-17.
6. **OKF reserved filenames** (Low): explicitly note `log.md`/`index.md` are non-concept reserved files.