Design Review
Purpose
Verifies the design document (the file at the path printed by goga history path -f design.md) for logical correctness by tracing the full code stack for each entry point and test scenario. This is a verification pass — the goal is to find logical errors before plan creation.
You do not write implementation code.
You trace each logical chain and find where the logic breaks.
Core Principle
Trace, do not assume. For each logical chain, mentally execute the code path step by step. Read actual source files and library documentation to verify correctness. If a step cannot be verified due to missing information — that in itself is a remark.
User Interaction Rule
Always offer answer choices. When requesting a user decision or confirmation — always provide 2–4 concrete options. Never ask open-ended questions without selectable choices.
CODEMANIFEST Editing
When tracing reveals errors in the contract itself (not in the design's interpretation of the contract), you must propose CODEMANIFEST edits. These are not design-level remarks — they are contract-level errors that block correct implementation regardless of design structure.
Conditions requiring CODEMANIFEST editing:
- Insufficient requirements: missing type declarations, incomplete signatures, absent method/property descriptions
- Contradictory requirements: contradictions between entity interfaces, type mismatches between interacting entities
- Contract interaction errors: broken type chains between interfaces,
Type:: mutations referencing non-existent or incompatible types, interface contracts diverging in data formats
All CODEMANIFEST edits must be proposed to the user before applying.
Phases
Phase 1: Context Loading
- Load the DSL specification and DSL application principles:
- Use the Skill tool to invoke
goga-cell — for understanding CODEMANIFEST DSL rules before reviewing contracts
(document structure, signature syntax, Imports rules, Usages rules, Annotations rules, types, mutations, embeddings, constraints)
- Use the Skill tool to invoke
goga-cookbook — for understanding cell design principles and CODEMANIFEST
(when to use Entity vs Routine, when to apply mutations and embeddings, usage file authoring principles, cell granularity)
- Read the design document from the path printed by
goga history path -f design.md
- Read all relevant CODEMANIFEST files referenced by the design
- Read existing source files referenced by the design (if any)
- Execute
goga schema --help to understand the command, then execute goga schema to obtain the full project dependency graph. Use --depends-on <cell_path> to discover cells that depend on cells modified by the design. This ensures the review covers all affected cells.
- Reading Usage specifications by reference: determine which Usages to read based on references:
- Always read root Usages — Usages (files from
.goga/usages/) referenced by global Annotations in the CODEMANIFEST header (via backtick syntax) are always read
- Always read Usages of modified entities — for each entity covered by the design, scan its annotations for references (backticks) to Usages. All referenced Usages are read
- Skip unreferenced Usages — Usage entries not referenced by global
Annotations AND not referenced by annotations of any covered entity are skipped
Phase 2: Code Stack Trace Verification
For each entry point (method, function, constructor) described in the design, perform a full stack trace.
Step 1. Chain Tracing
Trace the logical chain from entry to exit, step by step:
- Entry — what initiates this path, what is the calling context
- Input data — what data arrives, in what form, from where
- Input validation — whether input data is validated, what happens on invalid input
- Each transformation step — what changes, what types pass through, what is returned
- External calls — what libraries/imports are called, what they actually return
- State changes — what state is modified, what side effects occur
- Exit — what is returned, in what form, where it goes
Tracing rule: at each step, explicitly write down what data exists and in what form. Do not skip steps — "and then it works" is not an acceptable trace.
Contract interaction tracing — for each step where entities interact across boundaries:
Interface ↔ Type interaction: when entity A receives or returns a type from Imports/Usages, verify:
- the type is declared in the source CODEMANIFEST
- the type shape (fields, methods, properties) matches the entity's expectations
- on mismatch — record as a CODEMANIFEST issue (criticality: Critical)
Type ↔ Mutation interaction: for each Type:: mutation in the chain:
- verify that the base type exists with the correct name (alias from
Imports or qualified name from Usages)
- verify that declared methods/properties of the mutation target are compatible with the base type
- for multi-level mutations (
A::B::Cls) verify that each segment is resolvable and compatible
- if any segment is broken — record as a CODEMANIFEST issue (criticality: Critical)
Interface ↔ Interface interaction: when entity A calls entity B or passes data to it:
- verify that A's output type matches B's expected input type at the contract level (not just implementation)
- verify error type compatibility
- verify that shared type references resolve to the same actual type
- on mismatch — record as a CODEMANIFEST issue (criticality: High or Critical depending on impact)
Step 2. Checkpoint Verification
At each step in the chain, verify:
- Type continuity: does the output type of step N match the input type of step N+1?
- Logical correctness: does the transformation work correctly? (e.g., if filtering by "active" status, does the logic actually check for "active" and not something else?)
- Missing steps: is there a gap between what the contract requires and what the design provides?
- Error paths: what happens on error at this step? Is the error handled? Does it propagate correctly?
- Boundary cases at this step: what happens on empty input, None/null, wrong type, boundary values, concurrent access?
If a checkpoint fails — record a remark: exact location in the chain, what is wrong, what should happen.
Contract consistency checkpoints — additionally verify:
- Cross-entity type continuity: if step N in entity A outputs type T, and step M in entity B expects type T, does T mean the same thing in both contexts? (same
Imports source, same alias, same shape)
- Mutation contract consistency: if a
Type:: mutation creates a type used anywhere else, does the mutated contract satisfy all consumers?
- Annotation ↔ entity consistency: do annotations reference types/usages/parameters that actually exist in the CODEMANIFEST?
If a contract consistency checkpoint fails — classify as a CODEMANIFEST issue (the error is in the contract itself), not a design issue. Propose a CODEMANIFEST fix.
Step 3. External Dependency Verification
For each external call in the chain:
- Read the actual library documentation or Usage specification
- Verify that the called method/function exists and returns what the design assumes
- Verify calling convention correctness (argument order, named parameters, etc.)
- Verify that the return type matches the expectations of the next step
For each imported usage from Imports → Usages:
- Verify that the specified file exists at path
{from_path}/.usages/{usage_name}.md
- Read the imported usage file and verify that its content matches the design's assumptions
- Verify that the imported usage is used correctly (it provides practical guidelines, not contractual obligations)
- Ensure the design does not treat the imported usage as a type contract
For categorizing local usages in the design document:
- Verify that functional categories are semantically distinct — no two categories should cover the same domain
- Verify that practices placed in the same category file are actually related by function
- Verify that existing
.usages/ files in the cell have been reviewed and matching categories were extended, not duplicated
Phase 3: Test Logic Verification
For each test scenario described in the design, create a detailed test trace with the following structure for each test case.
Mandatory Format for Each Test Case
Each test case MUST be described by all 6 elements:
- Name:
<test name> — self-documenting, following target language conventions
- Setup: exact fixture configuration (tmp_path contents, mocks, patches) — specific code or description with exact values
- Input: exact values passed to the function/CLI (arguments, types, structure)
- Trace: step-by-step code execution with this exact input — what each helper receives, what it returns, what side effects occur at each step
- Assertions: specific checks with exact expected values (paths, contents, return codes, output strings) — written as actual assert statements or equivalent
- Sufficiency assessment: why this test is needed and what regression it prevents
Step 1. Positive Test Traces
For each positive test:
- Write a complete 6-element trace
- Verify that the expected result is correct based on the design logic traced in Phase 2
- Verify that test input is sufficient — not too trivial to catch real bugs
Step 2. Negative Test Traces
For each negative test:
- Write a complete 6-element trace
- Verify exactly where the code fails and that the error is handled correctly (correct error message, correct return code)
- Verify that the test checks the correct failure mode
Step 3. Boundary Case Coverage Audit
For each entity:
- Enumerate all boundary conditions:
- empty input, empty collection, empty string
- None/null values
- maximum size / boundary values
- wrong types
- concurrent access (if applicable)
- missing dependencies
- For each boundary condition: write a complete 6-element test trace or record a gap if it belongs in the test gaps section
Step 4. Test Data Sufficiency
For each test:
- Are the test data realistic enough to catch real bugs?
- Are there enough different test cases to cover the described behavior?
- Do the test data include typical usage patterns, not just trivial "happy path" cases?
Phase 4: Report and Resolve Remarks (Interactive)
Collect all remarks from Phases 2 and 3 before presenting them. Sort by criticality: Critical → High → Medium → Low → Test Gaps → CODEMANIFEST Issues.
Divide remarks into two categories:
- Design remarks — issues in the design document (logical errors, missing boundary cases, test gaps)
- CODEMANIFEST remarks — issues in the contract itself (insufficient/contradictory requirements, broken type chains, missing declarations)
Present remarks one at a time. For each remark:
Step 1. Show the Remark
Present one remark with:
- Category (Design / CODEMANIFEST)
- Criticality (Critical / High / Medium / Low / Test Gap)
- Location — exact reference to the design section or CODEMANIFEST file/entity
- What is wrong — clear description of the problem
- Proposed fix — the exact change needed, not vague advice. For CODEMANIFEST issues: show the exact DSL change. For test gaps: write the complete 6-element trace (name, setup, input, trace, assertions, sufficiency)
Step 2. Request User Decision
Use AskUserQuestion with choices:
- Apply the proposed fix — apply the fix to the design document or CODEMANIFEST immediately
- Propose an alternative — the user describes a different approach to fixing
Step 3. Apply the Decision
- Apply proposed fix (design): update the design document, then re-verify that the fix does not break other chains (re-trace affected chains). Briefly report the re-verification result.
- Apply proposed fix (CODEMANIFEST): edit the CODEMANIFEST file, re-run the linter:
goga lint. If the linter reports errors — fix the DSL syntax. Re-verify affected design chains against the updated contract.
- Skip: record the remark as "skipped" and continue.
- Propose an alternative: discuss the alternative with the user, agree on a fix, apply it, re-trace affected chains.
Step 4. Proceed to Next Remark
Repeat from Step 1 for the next remark. Show a brief counter: "Remark 3 of 12".
After processing all remarks, show the summary:
- Fixed: N remarks (breakdown by criticality, split by design/CODEMANIFEST)
- Skipped: N remarks (breakdown by criticality, split by design/CODEMANIFEST)
- Design document status: updated / unchanged
- CODEMANIFEST status: updated (list of files) / unchanged
Output
- Remark summary: count of fixed / skipped by criticality
- Updated design document (if fixes were applied)
Final Self-Check
Before completion, verify:
- Was the DSL specification loaded via
goga-cell and goga-cookbook skills before reviewing contracts?
- Was each entry point in the design traced through the full code stack?
- Was each checkpoint in each chain verified (type, logic, error, boundary case)?
- Were contract interaction checkpoints verified (interface ↔ type, type ↔ mutation, interface ↔ interface)?
- Were external dependencies verified against actual documentation?
- Were imported usages from
Imports → Usages verified (file exists, content matches design assumptions)?
- Was each test scenario traced (positive, negative, boundary)?
- Was test data sufficiency verified for each test?
- Was each remark presented one at a time with a fix decision?
- Were approved fixes applied to the design document or CODEMANIFEST?
- Were CODEMANIFEST changes re-verified by the linter?
- Were affected chains re-traced after each fix?
- Were summary totals of fixed/skipped remarks provided (split by design/CODEMANIFEST)?
- Were CODEMANIFEST issues separated from design issues and proposed with specific DSL fixes?
If at least one answer is "no" — complete the missing verification before returning.
1---2name: goga-review-design3description: Design document verification via code stack tracing4---5# Design Review67## Purpose89Verifies the design document (the file at the path printed by `goga history path -f design.md`) for **logical correctness** by tracing the full code stack for each entry point and test scenario. This is a **verification pass** — the goal is to find logical errors before plan creation.1011You do **not** write implementation code.12You **trace** each logical chain and **find** where the logic breaks.1314---1516## Core Principle1718**Trace, do not assume.** For each logical chain, mentally execute the code path step by step. Read actual source files and library documentation to verify correctness. If a step cannot be verified due to missing information — that in itself is a remark.1920### User Interaction Rule2122**Always offer answer choices.** When requesting a user decision or confirmation — always provide 2–4 concrete options. Never ask open-ended questions without selectable choices.2324### CODEMANIFEST Editing2526When tracing reveals errors **in the contract itself** (not in the design's interpretation of the contract), you **must** propose CODEMANIFEST edits. These are not design-level remarks — they are contract-level errors that block correct implementation regardless of design structure.2728Conditions requiring CODEMANIFEST editing:29- **Insufficient requirements**: missing type declarations, incomplete signatures, absent method/property descriptions30- **Contradictory requirements**: contradictions between entity interfaces, type mismatches between interacting entities31- **Contract interaction errors**: broken type chains between interfaces, `Type::` mutations referencing non-existent or incompatible types, interface contracts diverging in data formats3233All CODEMANIFEST edits must be **proposed to the user** before applying.3435---3637## Phases3839### Phase 1: Context Loading40411. Load the DSL specification and DSL application principles:42 - Use the **Skill tool** to invoke `goga-cell` — for understanding CODEMANIFEST DSL rules before reviewing contracts43 (document structure, signature syntax, Imports rules, Usages rules, Annotations rules, types, mutations, embeddings, constraints)44 - Use the **Skill tool** to invoke `goga-cookbook` — for understanding cell design principles and CODEMANIFEST45 (when to use Entity vs Routine, when to apply mutations and embeddings, usage file authoring principles, cell granularity)462. Read the design document from the path printed by `goga history path -f design.md`473. Read all relevant CODEMANIFEST files referenced by the design484. Read existing source files referenced by the design (if any)495. Execute `goga schema --help` to understand the command, then execute `goga schema` to obtain the full project dependency graph. Use `--depends-on <cell_path>` to discover cells that depend on cells modified by the design. This ensures the review covers all affected cells.506. **Reading Usage specifications by reference**: determine which Usages to read based on references:51 - **Always read root Usages** — Usages (files from `.goga/usages/`) referenced by global `Annotations` in the CODEMANIFEST header (via backtick syntax) are always read52 - **Always read Usages of modified entities** — for each entity covered by the design, scan its annotations for references (backticks) to Usages. All referenced Usages are read53 - **Skip unreferenced Usages** — Usage entries not referenced by global `Annotations` AND not referenced by annotations of any covered entity are skipped5455---5657### Phase 2: Code Stack Trace Verification5859For **each entry point** (method, function, constructor) described in the design, perform a full stack trace.6061#### Step 1. Chain Tracing6263Trace the logical chain from entry to exit, step by step:64651. **Entry** — what initiates this path, what is the calling context662. **Input data** — what data arrives, in what form, from where673. **Input validation** — whether input data is validated, what happens on invalid input684. **Each transformation step** — what changes, what types pass through, what is returned695. **External calls** — what libraries/imports are called, what they actually return706. **State changes** — what state is modified, what side effects occur717. **Exit** — what is returned, in what form, where it goes7273**Tracing rule**: at each step, explicitly write down what data exists and in what form. Do not skip steps — "and then it works" is not an acceptable trace.7475**Contract interaction tracing** — for each step where entities interact across boundaries:76771. **Interface ↔ Type interaction**: when entity A receives or returns a type from `Imports`/`Usages`, verify:78 - the type is declared in the source CODEMANIFEST79 - the type shape (fields, methods, properties) matches the entity's expectations80 - on mismatch — record as a **CODEMANIFEST issue** (criticality: Critical)81822. **Type ↔ Mutation interaction**: for each `Type::` mutation in the chain:83 - verify that the base type exists with the correct name (alias from `Imports` or qualified name from `Usages`)84 - verify that declared methods/properties of the mutation target are compatible with the base type85 - for multi-level mutations (`A::B::Cls`) verify that each segment is resolvable and compatible86 - if any segment is broken — record as a **CODEMANIFEST issue** (criticality: Critical)87883. **Interface ↔ Interface interaction**: when entity A calls entity B or passes data to it:89 - verify that A's output type matches B's expected input type at the **contract level** (not just implementation)90 - verify error type compatibility91 - verify that shared type references resolve to the same actual type92 - on mismatch — record as a **CODEMANIFEST issue** (criticality: High or Critical depending on impact)9394#### Step 2. Checkpoint Verification9596At **each step** in the chain, verify:9798- **Type continuity**: does the output type of step N match the input type of step N+1?99- **Logical correctness**: does the transformation work correctly? (e.g., if filtering by "active" status, does the logic actually check for "active" and not something else?)100- **Missing steps**: is there a gap between what the contract requires and what the design provides?101- **Error paths**: what happens on error at this step? Is the error handled? Does it propagate correctly?102- **Boundary cases at this step**: what happens on empty input, None/null, wrong type, boundary values, concurrent access?103104If a checkpoint fails — record a remark: exact location in the chain, what is wrong, what should happen.105106**Contract consistency checkpoints** — additionally verify:107- **Cross-entity type continuity**: if step N in entity A outputs type T, and step M in entity B expects type T, does T mean the same thing in both contexts? (same `Imports` source, same alias, same shape)108- **Mutation contract consistency**: if a `Type::` mutation creates a type used anywhere else, does the mutated contract satisfy all consumers?109- **Annotation ↔ entity consistency**: do annotations reference types/usages/parameters that actually exist in the CODEMANIFEST?110111If a contract consistency checkpoint fails — classify as a **CODEMANIFEST issue** (the error is in the contract itself), not a design issue. Propose a CODEMANIFEST fix.112113#### Step 3. External Dependency Verification114115For each external call in the chain:116117- Read the actual library documentation or Usage specification118- Verify that the called method/function exists and returns what the design assumes119- Verify calling convention correctness (argument order, named parameters, etc.)120- Verify that the return type matches the expectations of the next step121122For each imported usage from `Imports` → `Usages`:123124- Verify that the specified file exists at path `{from_path}/.usages/{usage_name}.md`125- Read the imported usage file and verify that its content matches the design's assumptions126- Verify that the imported usage is used correctly (it provides practical guidelines, not contractual obligations)127- Ensure the design does not treat the imported usage as a type contract128129For categorizing local usages in the design document:130131- Verify that functional categories are semantically distinct — no two categories should cover the same domain132- Verify that practices placed in the same category file are actually related by function133- Verify that existing `.usages/` files in the cell have been reviewed and matching categories were extended, not duplicated134135---136137### Phase 3: Test Logic Verification138139For **each test scenario** described in the design, create a **detailed test trace** with the following structure for each test case.140141#### Mandatory Format for Each Test Case142143Each test case MUST be described by all 6 elements:1441451. **Name**: `<test name>` — self-documenting, following target language conventions1462. **Setup**: exact fixture configuration (tmp_path contents, mocks, patches) — specific code or description with exact values1473. **Input**: exact values passed to the function/CLI (arguments, types, structure)1484. **Trace**: step-by-step code execution with this exact input — what each helper receives, what it returns, what side effects occur at each step1495. **Assertions**: specific checks with exact expected values (paths, contents, return codes, output strings) — written as actual assert statements or equivalent1506. **Sufficiency assessment**: why this test is needed and what regression it prevents151152#### Step 1. Positive Test Traces153154For each positive test:1551. Write a complete 6-element trace1562. Verify that the expected result is **correct** based on the design logic traced in Phase 21573. Verify that test input is sufficient — not too trivial to catch real bugs158159#### Step 2. Negative Test Traces160161For each negative test:1621. Write a complete 6-element trace1632. Verify exactly where the code fails and that the error is handled correctly (correct error message, correct return code)1643. Verify that the test checks the **correct** failure mode165166#### Step 3. Boundary Case Coverage Audit167168For each entity:1691. Enumerate all boundary conditions:170 - empty input, empty collection, empty string171 - None/null values172 - maximum size / boundary values173 - wrong types174 - concurrent access (if applicable)175 - missing dependencies1762. For each boundary condition: write a complete 6-element test trace or record a gap if it belongs in the test gaps section177178#### Step 4. Test Data Sufficiency179180For each test:1811. Are the test data realistic enough to catch real bugs?1822. Are there enough different test cases to cover the described behavior?1833. Do the test data include typical usage patterns, not just trivial "happy path" cases?184185---186187### Phase 4: Report and Resolve Remarks (Interactive)188189Collect all remarks from Phases 2 and 3 before presenting them. Sort by criticality: Critical → High → Medium → Low → Test Gaps → CODEMANIFEST Issues.190191Divide remarks into two categories:192- **Design remarks** — issues in the design document (logical errors, missing boundary cases, test gaps)193- **CODEMANIFEST remarks** — issues in the contract itself (insufficient/contradictory requirements, broken type chains, missing declarations)194195Present remarks **one at a time**. For each remark:196197#### Step 1. Show the Remark198199Present one remark with:200201- **Category** (Design / CODEMANIFEST)202- **Criticality** (Critical / High / Medium / Low / Test Gap)203- **Location** — exact reference to the design section or CODEMANIFEST file/entity204- **What is wrong** — clear description of the problem205- **Proposed fix** — the exact change needed, not vague advice. For CODEMANIFEST issues: show the exact DSL change. For test gaps: write the complete 6-element trace (name, setup, input, trace, assertions, sufficiency)206207#### Step 2. Request User Decision208209Use AskUserQuestion with choices:2102111. **Apply the proposed fix** — apply the fix to the design document or CODEMANIFEST immediately2122. **Propose an alternative** — the user describes a different approach to fixing213214#### Step 3. Apply the Decision215216- **Apply proposed fix (design)**: update the design document, then re-verify that the fix does not break other chains (re-trace affected chains). Briefly report the re-verification result.217- **Apply proposed fix (CODEMANIFEST)**: edit the CODEMANIFEST file, re-run the linter: `goga lint`. If the linter reports errors — fix the DSL syntax. Re-verify affected design chains against the updated contract.218- **Skip**: record the remark as "skipped" and continue.219- **Propose an alternative**: discuss the alternative with the user, agree on a fix, apply it, re-trace affected chains.220221#### Step 4. Proceed to Next Remark222223Repeat from Step 1 for the next remark. Show a brief counter: "Remark 3 of 12".224225After processing all remarks, show the summary:226227- **Fixed**: N remarks (breakdown by criticality, split by design/CODEMANIFEST)228- **Skipped**: N remarks (breakdown by criticality, split by design/CODEMANIFEST)229- **Design document status**: updated / unchanged230- **CODEMANIFEST status**: updated (list of files) / unchanged231232---233234## Output235236- Remark summary: count of fixed / skipped by criticality237- Updated design document (if fixes were applied)238239---240241## Final Self-Check242243Before completion, verify:2442451. Was the DSL specification loaded via `goga-cell` and `goga-cookbook` skills before reviewing contracts?2462. Was each entry point in the design traced through the full code stack?2473. Was each checkpoint in each chain verified (type, logic, error, boundary case)?2484. Were contract interaction checkpoints verified (interface ↔ type, type ↔ mutation, interface ↔ interface)?2495. Were external dependencies verified against actual documentation?2506. Were imported usages from `Imports` → `Usages` verified (file exists, content matches design assumptions)?2517. Was each test scenario traced (positive, negative, boundary)?2528. Was test data sufficiency verified for each test?2539. Was each remark presented one at a time with a fix decision?25410. Were approved fixes applied to the design document or CODEMANIFEST?25511. Were CODEMANIFEST changes re-verified by the linter?25612. Were affected chains re-traced after each fix?25713. Were summary totals of fixed/skipped remarks provided (split by design/CODEMANIFEST)?25814. Were CODEMANIFEST issues separated from design issues and proposed with specific DSL fixes?259260If at least one answer is "no" — complete the missing verification before returning.261262---