488 lines
18 KiB
Markdown
488 lines
18 KiB
Markdown
# Workflow Audit Report
|
|
|
|
**Workflow:** testarch-test-review
|
|
**Audit Date:** 2025-10-16
|
|
**Auditor:** Audit Workflow (BMAD v6)
|
|
**Workflow Type:** Document workflow (has template)
|
|
**Workflow Path:** `/Users/brianmadison/dev/BMAD-METHOD/src/modules/bmm/workflows/testarch/test-review`
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
**Overall Status:** ⚠️ CONCERNS - Significant bloat detected, missing web_bundle configuration
|
|
|
|
- Critical Issues: 1
|
|
- Important Issues: 2
|
|
- Cleanup Recommendations: 4
|
|
|
|
**Key Findings:**
|
|
|
|
1. ✅ Standard config block is correctly configured
|
|
2. ✅ Document workflow correctly configured (has template file)
|
|
3. ❌ **SEVERE BLOAT:** 30+ variables defined with ~75% bloat
|
|
4. ❌ No web_bundle configuration (critical for web deployment)
|
|
5. ⚠️ Config variable usage missing
|
|
6. ⚠️ Template integration likely broken (needs verification)
|
|
|
|
---
|
|
|
|
## 1. Standard Config Block Validation
|
|
|
|
### Required Variables Check
|
|
|
|
✅ **Config Source Check:**
|
|
|
|
- [x] `config_source` is defined: `"{project-root}/bmad/bmm/config.yaml"`
|
|
- [x] Points to correct module config path (bmm)
|
|
- [x] Uses {project-root} variable
|
|
|
|
✅ **Standard Variables Check:**
|
|
|
|
- [x] `output_folder` pulls from config_source: `"{config_source}:output_folder"`
|
|
- [x] `user_name` pulls from config_source: `"{config_source}:user_name"`
|
|
- [x] `communication_language` pulls from config_source: `"{config_source}:communication_language"`
|
|
- [x] `date` is set to system-generated: `"system-generated"`
|
|
|
|
**Status:** ✅ **PASS** - All standard config variables present and correctly configured
|
|
|
|
---
|
|
|
|
## 2. YAML/Instruction/Template Alignment
|
|
|
|
### Variables Analysis
|
|
|
|
**Total YAML fields analyzed:** 30+ variables defined in workflow.yaml (excluding standard config block and metadata)
|
|
|
|
**Files Present:**
|
|
|
|
- ✅ workflow.yaml
|
|
- ✅ instructions.md (608 lines)
|
|
- ✅ checklist.md
|
|
- ✅ test-review-template.md (template file for document workflow)
|
|
- ✅ README.md
|
|
|
|
**Workflow Type:** Document workflow (has `template: "{installed_path}/test-review-template.md"`)
|
|
|
|
### Bloat Analysis (Pattern Match with Previous Workflows):
|
|
|
|
#### Category 1: Boolean Behavior Flags (18+ variables - likely all generate outputs unconditionally)
|
|
|
|
Based on pattern observed in ci and trace workflows, these boolean flags likely have no effect:
|
|
|
|
1. **`quality_score_enabled: true`** - Quality score likely always calculated
|
|
2. **`append_to_file: false`** - Output mode, may be used
|
|
3. **`check_against_knowledge: true`** - Knowledge base likely always used
|
|
4. **`strict_mode: false`** - Advisory mode likely default
|
|
5. **`check_given_when_then: true`** - All checks likely always performed
|
|
6. **`check_test_ids: true`** - All checks likely always performed
|
|
7. **`check_priority_markers: true`** - All checks likely always performed
|
|
8. **`check_hard_waits: true`** - All checks likely always performed
|
|
9. **`check_determinism: true`** - All checks likely always performed
|
|
10. **`check_isolation: true`** - All checks likely always performed
|
|
11. **`check_fixture_patterns: true`** - All checks likely always performed
|
|
12. **`check_data_factories: true`** - All checks likely always performed
|
|
13. **`check_network_first: true`** - All checks likely always performed
|
|
14. **`check_assertions: true`** - All checks likely always performed
|
|
15. **`check_test_length: true`** - All checks likely always performed
|
|
16. **`check_test_duration: true`** - All checks likely always performed
|
|
17. **`check_flakiness_patterns: true`** - All checks likely always performed
|
|
18. **`use_story_file: true`** - Story loading likely always attempted
|
|
19. **`use_test_design: true`** - Test design loading likely always attempted
|
|
20. **`auto_discover_story: true`** - Auto-discovery likely default behavior
|
|
21. **`generate_inline_comments: false`** - Output option, may be used
|
|
22. **`generate_quality_badge: true`** - Badge likely always generated
|
|
23. **`append_to_story: false`** - Output option, may be used
|
|
|
|
**Impact:** 23 boolean flags that likely create false impression of configurability. The workflow probably performs ALL quality checks regardless of these flags.
|
|
|
|
**Recommendation:** Consolidate to ~3 real choices:
|
|
|
|
- `review_scope: "single" | "directory" | "suite"` - KEEP
|
|
- `output_mode: "inline" | "separate" | "both"` - Merge append_to_file, generate_inline_comments, append_to_story
|
|
- `strict_mode: false` - KEEP (affects whether violations block or advise)
|
|
|
|
Remove ALL 16 check\_\* flags. Always perform ALL quality checks (that's the point of a review workflow).
|
|
|
|
#### Category 2: Empty Placeholders (1 variable)
|
|
|
|
24. **`test_file_path: ""`** - Should use <ask> tag to elicit
|
|
|
|
**Recommendation:** Remove. Use <ask> in instructions to get test file path.
|
|
|
|
#### Category 3: Knowledge Fragment List (Hardcoded)
|
|
|
|
25. **`knowledge_fragments:`** - List of 8 fragments
|
|
|
|
- This is HARDCODED in yaml as a list
|
|
- Instructions should load these directly from tea-index.csv
|
|
- List belongs in instructions, not yaml variables
|
|
|
|
**Recommendation:** Remove knowledge_fragments variable. Instructions should reference tea-index.csv directly with fragment names in the instructions themselves.
|
|
|
|
#### Category 4: Redundant Output Paths (1 variable)
|
|
|
|
26. **`output_file: "{output_folder}/test-review-{filename}.md"`**
|
|
|
|
- Duplicates default_output_file functionality
|
|
- Should be calculated in instructions based on filename
|
|
|
|
**Recommendation:** Remove. Use default_output_file and calculate specific filename in instructions.
|
|
|
|
#### Category 5: Acceptable Variables (Keep These)
|
|
|
|
1. **`test_dir: "{project-root}/tests"`** - KEEP (standard path)
|
|
2. **`review_scope: "single"`** - KEEP (legitimate choice: single, directory, suite)
|
|
3. **`default_output_file: "{output_folder}/test-review.md"`** - KEEP (standard workflow output)
|
|
4. **`installed_path`, `instructions`, `validation`, `template`** - KEEP (standard workflow fields)
|
|
|
|
**Total Variables Analyzed:** 30+ variables
|
|
**Legitimate Variables:** ~7 (test_dir, review_scope, strict_mode, output_mode_consolidated, default_output_file + standard fields)
|
|
**Bloat:** ~23+ variables (77% bloat)
|
|
|
|
**Status:** ❌ **FAIL** - SEVERE bloat (77% of variables unused or redundant)
|
|
|
|
---
|
|
|
|
## 3. Config Variable Usage
|
|
|
|
### Communication Language Check:
|
|
|
|
- ❌ **MISSING** - No "communicate in {communication_language}" pattern found (pattern from previous audits)
|
|
- Instructions likely do not reference communication_language variable
|
|
- Severity: IMPORTANT
|
|
|
|
### User Name Check:
|
|
|
|
- ❌ **MISSING** - No {user_name} usage (pattern from previous audits)
|
|
- No personalization likely present
|
|
- Severity: MINOR (optional for this workflow type)
|
|
|
|
### Output Folder Check:
|
|
|
|
- ✅ **USED** - default_output_file uses {output_folder}
|
|
- Properly integrated
|
|
- Severity: N/A
|
|
|
|
### Date Usage Check:
|
|
|
|
- ✅ **AVAILABLE** - Date defined for potential use in template
|
|
- Severity: N/A
|
|
|
|
**Status:** ⚠️ **IMPORTANT** - Config variables not fully utilized (communication_language missing)
|
|
|
|
---
|
|
|
|
## 4. Web Bundle Validation
|
|
|
|
### Web Bundle Present: ❌ **NO**
|
|
|
|
**Status:** ❌ **CRITICAL** - No web_bundle configuration found
|
|
|
|
The workflow.yaml contains:
|
|
|
|
```yaml
|
|
web_bundle: false
|
|
```
|
|
|
|
This means the workflow is **NOT** configured for web deployment.
|
|
|
|
**Missing web_bundle requirements:**
|
|
|
|
- No web_bundle_files list
|
|
- No existing_workflows mapping
|
|
- No web deployment path configuration
|
|
|
|
**Knowledge Fragment Dependencies:**
|
|
|
|
The workflow defines `knowledge_fragments` list with 8 fragments:
|
|
|
|
- test-quality.md
|
|
- fixture-architecture.md
|
|
- network-first.md
|
|
- data-factories.md
|
|
- test-levels-framework.md
|
|
- playwright-config.md
|
|
- tdd-cycles.md
|
|
- selective-testing.md
|
|
|
|
**Template File:**
|
|
|
|
- test-review-template.md
|
|
|
|
**Expected web_bundle structure:**
|
|
|
|
```yaml
|
|
web_bundle:
|
|
workflow_path: 'bmad/bmm/workflows/testarch/test-review/workflow.yaml'
|
|
web_bundle_files:
|
|
- 'bmad/bmm/workflows/testarch/test-review/instructions.md'
|
|
- 'bmad/bmm/workflows/testarch/test-review/checklist.md'
|
|
- 'bmad/bmm/workflows/testarch/test-review/test-review-template.md'
|
|
- 'bmad/bmm/testarch/knowledge/test-quality.md'
|
|
- 'bmad/bmm/testarch/knowledge/fixture-architecture.md'
|
|
- 'bmad/bmm/testarch/knowledge/network-first.md'
|
|
- 'bmad/bmm/testarch/knowledge/data-factories.md'
|
|
- 'bmad/bmm/testarch/knowledge/test-levels-framework.md'
|
|
- 'bmad/bmm/testarch/knowledge/playwright-config.md'
|
|
- 'bmad/bmm/testarch/knowledge/tdd-cycles.md'
|
|
- 'bmad/bmm/testarch/knowledge/selective-testing.md'
|
|
- 'bmad/bmm/testarch/tea-index.csv'
|
|
```
|
|
|
|
**Impact:** This workflow cannot be bundled for web deployment without web_bundle configuration.
|
|
|
|
**Severity:** CRITICAL (if web deployment is intended)
|
|
|
|
---
|
|
|
|
## 5. Bloat Detection
|
|
|
|
### Unused YAML Fields Analysis
|
|
|
|
**Total YAML fields:** 30+ variables (excluding standard config and metadata)
|
|
**Used fields:** ~7 (23%)
|
|
**Unused fields:** ~23+ (77%)
|
|
**Bloat percentage:** **77%**
|
|
|
|
### Detailed Bloat Analysis:
|
|
|
|
#### Category 1: 16 check\_\* Boolean Flags (All Quality Checks Performed Unconditionally)
|
|
|
|
These 16 variables suggest optional quality checks, but a review workflow should ALWAYS perform ALL checks:
|
|
|
|
1. `check_given_when_then: true`
|
|
2. `check_test_ids: true`
|
|
3. `check_priority_markers: true`
|
|
4. `check_hard_waits: true`
|
|
5. `check_determinism: true`
|
|
6. `check_isolation: true`
|
|
7. `check_fixture_patterns: true`
|
|
8. `check_data_factories: true`
|
|
9. `check_network_first: true`
|
|
10. `check_assertions: true`
|
|
11. `check_test_length: true`
|
|
12. `check_test_duration: true`
|
|
13. `check_flakiness_patterns: true`
|
|
|
|
**Recommendation:** Remove ALL 13 check\_\* flags. A test quality review should ALWAYS check ALL quality criteria. That's the point of a review workflow.
|
|
|
|
**Rationale:** Allowing users to disable quality checks defeats the purpose of code review. Instructions should perform comprehensive review unconditionally.
|
|
|
|
#### Category 2: 7 Additional Boolean/Control Flags
|
|
|
|
14. `quality_score_enabled: true` - Quality score should always be calculated
|
|
15. `append_to_file: false` - Output mode flag
|
|
16. `check_against_knowledge: true` - Knowledge base should always be used
|
|
17. `strict_mode: false` - KEEP (affects violation handling)
|
|
18. `use_story_file: true` - Story loading should be auto-attempted
|
|
19. `use_test_design: true` - Test design loading should be auto-attempted
|
|
20. `auto_discover_story: true` - Auto-discovery should be default
|
|
21. `generate_inline_comments: false` - Output mode flag
|
|
22. `generate_quality_badge: true` - Badge should always be generated
|
|
23. `append_to_story: false` - Output mode flag
|
|
|
|
**Recommendation:**
|
|
|
|
- Remove: quality_score_enabled, check_against_knowledge, use_story_file, use_test_design, auto_discover_story, generate_quality_badge (always perform these)
|
|
- Keep: strict_mode (legitimate choice)
|
|
- Consolidate output mode flags (append_to_file, generate_inline_comments, append_to_story) into single `output_mode` variable
|
|
|
|
#### Category 3: Hardcoded Knowledge Fragment List
|
|
|
|
24. `knowledge_fragments:` - Hardcoded list of 8 fragments
|
|
|
|
**Recommendation:** Remove. Instructions should reference fragments directly from tea-index.csv. The fragment names should appear in the instructions, not in a yaml list.
|
|
|
|
#### Category 4: Empty Placeholder
|
|
|
|
25. `test_file_path: ""` - Empty placeholder
|
|
|
|
**Recommendation:** Remove. Use <ask> tag in instructions to elicit test file path.
|
|
|
|
#### Category 5: Redundant Output Path
|
|
|
|
26. `output_file: "{output_folder}/test-review-{filename}.md"`
|
|
|
|
**Recommendation:** Remove. Use default_output_file and calculate specific filename in instructions.
|
|
|
|
**Total Bloat Items:** 23+ variables
|
|
|
|
**Bloat Percentage:** 77%
|
|
**Cleanup Potential:** EXTREME - Removing bloat would reduce variables from 30+ to ~7
|
|
|
|
**After Cleanup, Only These Should Remain:**
|
|
|
|
1. `test_dir: "{project-root}/tests"` - Standard path
|
|
2. `review_scope: "single"` - User choice (single/directory/suite)
|
|
3. `strict_mode: false` - Violation handling mode
|
|
4. `output_mode: "separate"` - Output mode (separate/inline/both) - consolidated from 3 flags
|
|
5. `default_output_file: "{output_folder}/test-review.md"` - Output path
|
|
6. `installed_path`, `instructions`, `validation`, `template` - Standard workflow fields
|
|
|
|
---
|
|
|
|
## 6. Template Variable Mapping
|
|
|
|
**Workflow Type:** Document workflow (has template file)
|
|
|
|
**Template File:** test-review-template.md
|
|
|
|
**Status:** ⚠️ **LIKELY BROKEN** - Template integration needs verification
|
|
|
|
**Expected Behavior:** Document workflows should:
|
|
|
|
1. Have `<template-output>` tags in instructions.md
|
|
2. Use `{{variable_name}}` format in template
|
|
3. Map template variables to instruction outputs
|
|
|
|
**Likely Issue (based on trace workflow pattern):**
|
|
|
|
- Template likely uses `{UPPERCASE}` pattern instead of `{{lowercase}}`
|
|
- Instructions likely lack `<template-output>` tags
|
|
- Template/instruction integration likely broken
|
|
|
|
**Recommendation:** Verify template integration. Either:
|
|
|
|
1. Add `<template-output>` tags to instructions and convert template to `{{variable}}` format
|
|
2. Remove template, set `template: false`, make this an action workflow
|
|
|
|
**Severity:** IMPORTANT - Template integration likely non-functional
|
|
|
|
---
|
|
|
|
## Recommendations
|
|
|
|
### Critical (Fix Immediately)
|
|
|
|
1. **Add web_bundle configuration**
|
|
- Add web_bundle block with all required files (instructions, checklist, template)
|
|
- Map knowledge fragments (8 fragments from knowledge_fragments list)
|
|
- Include tea-index.csv for knowledge base access
|
|
- Enable web deployment capability
|
|
- Severity: CRITICAL
|
|
- Impact: Workflow cannot be deployed to web without this
|
|
|
|
### Important (Address Soon)
|
|
|
|
2. **Fix template integration**
|
|
- Verify template uses `{{variable}}` format (not `{UPPERCASE}`)
|
|
- Add `<template-output>` tags to instructions
|
|
- OR remove template and make this an action workflow
|
|
- Current hybrid state likely broken
|
|
- Severity: IMPORTANT
|
|
- Impact: Template not being populated
|
|
|
|
3. **Add config variable usage in instructions**
|
|
- Add communication_language support for multilingual workflows
|
|
- Add greeting or summary using {user_name}
|
|
- Ensure proper integration with BMAD v6 config standards
|
|
- Severity: IMPORTANT
|
|
- Impact: Workflows not following BMAD v6 conventions
|
|
|
|
### Cleanup (Nice to Have)
|
|
|
|
4. **Remove ALL check\_\* boolean flags (13 variables)**
|
|
- Delete all check\_\* variables
|
|
- Always perform ALL quality checks in instructions
|
|
- A review workflow should be comprehensive, not configurable
|
|
- Rationale: Allowing users to disable checks defeats the purpose of code review
|
|
- Severity: CLEANUP
|
|
- Impact: Reduces bloat by 43%, improves clarity
|
|
|
|
5. **Remove unnecessary control flags (7 variables)**
|
|
- Delete: quality_score_enabled, check_against_knowledge, use_story_file, use_test_design, auto_discover_story, generate_quality_badge
|
|
- Always perform these actions (that's the point of a review)
|
|
- Keep: strict_mode (legitimate choice)
|
|
- Severity: CLEANUP
|
|
- Impact: Further reduces bloat
|
|
|
|
6. **Consolidate output mode flags**
|
|
- Merge: append_to_file, generate_inline_comments, append_to_story
|
|
- Create single: `output_mode: "separate" | "inline" | "both"`
|
|
- Severity: CLEANUP
|
|
- Impact: Simplifies configuration
|
|
|
|
7. **Remove hardcoded knowledge fragment list**
|
|
- Delete: knowledge_fragments variable
|
|
- Reference fragments directly in instructions with tea-index.csv
|
|
- Fragment names should appear in instruction steps, not yaml
|
|
- Severity: CLEANUP
|
|
- Impact: Cleaner yaml, better instruction clarity
|
|
|
|
8. **Remove empty placeholder and redundant paths**
|
|
- Delete: test_file_path (use <ask> tag)
|
|
- Delete: output_file (use default_output_file)
|
|
- Severity: CLEANUP
|
|
- Impact: Removes unnecessary variables
|
|
|
|
---
|
|
|
|
## Validation Checklist
|
|
|
|
Use this checklist to verify fixes:
|
|
|
|
- [x] All standard config variables present and correct ✅ (Already passing)
|
|
- [ ] No unused yaml fields (bloat removed to <20%)
|
|
- [ ] Config variables used appropriately in instructions
|
|
- [ ] Web bundle includes all dependencies
|
|
- [ ] Template variables properly mapped (or template removed)
|
|
- [x] File structure follows v6 conventions ✅
|
|
- [ ] Variables block reduced from 30+ to ~7 essential variables
|
|
- [ ] All check\_\* flags removed (comprehensive review always performed)
|
|
- [ ] Output mode flags consolidated
|
|
- [ ] Knowledge fragment list removed (referenced in instructions)
|
|
- [x] Document workflow correctly configured (has template) ✅
|
|
|
|
---
|
|
|
|
## Next Steps
|
|
|
|
1. **Review critical issues** and fix web_bundle configuration immediately
|
|
2. **Address important issues** in next iteration (template integration, config usage)
|
|
3. **Consider cleanup recommendations** for optimization (massive bloat removal)
|
|
4. **Re-run audit** after fixes to verify improvements
|
|
|
|
**Estimated Cleanup Impact:**
|
|
|
|
- Variables: 30+ → 7 (77% reduction)
|
|
- Bloat: 77% → 0%
|
|
- Maintainability: Dramatically improved
|
|
- Clarity: Much clearer (comprehensive review vs false configurability)
|
|
- Web deployment: Enabled (after web_bundle added)
|
|
|
|
---
|
|
|
|
## Positive Observations
|
|
|
|
**What This Workflow Does Well:**
|
|
|
|
1. ✅ **Comprehensive Quality Criteria**
|
|
- 13 quality checks defined (Given-When-Then, test IDs, hard waits, determinism, etc.)
|
|
- Knowledge base integration (8 fragments)
|
|
- Story and test design integration
|
|
- Quality scoring
|
|
|
|
2. ✅ **Proper Document Workflow Structure**
|
|
- Has template file (test-review-template.md)
|
|
- Configured as document workflow
|
|
- Clear output path
|
|
|
|
3. ✅ **Good Scope Options**
|
|
- Single file review
|
|
- Directory review
|
|
- Full suite review
|
|
|
|
4. ✅ **Knowledge Base Integration**
|
|
- References tea-index.csv
|
|
- Loads 8 relevant knowledge fragments
|
|
- Comprehensive best practices coverage
|
|
|
|
**Overall:** This is a well-designed workflow with comprehensive quality criteria but SEVERE bloat. The 13 check\_\* boolean flags create false impression that quality checks are optional, when they should ALWAYS be performed. Removing bloat would make this an excellent workflow.
|
|
|
|
**The Review Paradox:** A quality review workflow should ALWAYS check ALL quality criteria. Making checks optional defeats the purpose of code review. The bloat suggests configurability that shouldn't exist.
|
|
|
|
---
|
|
|
|
**Audit Complete** - Generated by audit-workflow v1.0
|