Skip to content

Commit 681136e

Browse files
committed
fix: address PR review — workflow category, active-parent rule, config validation
- Add `workflow` rule category for process discipline checks; move active-outcome/opportunity-count rules from coherence to workflow - Add `active-node-parent-active` workflow rule: an active node's parent must also be active - Update `solution-quantity` best-practice to only apply when opportunity status is exploring or active - Check `allowSelfRef ⊆ hierarchy` in validate command, reporting mismatches as config errors with a new configErrors result field - Rename schemaErrors/schemaValidCount → nodeErrors/validCount for clarity - Update docs/rules.md: add workflow category, clarify scope is orthogonal to category, update examples
1 parent 3b3b6f9 commit 681136e

6 files changed

Lines changed: 161 additions & 71 deletions

File tree

docs/rules.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@ For how rules fit into the broader schema metadata, see [docs/schemas.md](schema
66

77
## Rule Categories
88

9-
Rules are grouped into three categories under `_metadata.rules`.
9+
Rules are grouped into categories under `_metadata.rules`. Categories are informational — they determine how violations are labelled and grouped in output, but do not affect how the rule is evaluated. Use `scope` to control evaluation mode.
1010

1111
| Category | Purpose |
1212
|---|---|
1313
| `validation` | Structural correctness — a violation means the node is incorrect and should be fixed |
14-
| `coherence` | Cross-node checks — for flagging conflicts or contradictions (often combined with `scope: 'global'` for multi-node/aggregate checks) |
14+
| `coherence` | Cross-node checks — for flagging conflicts or contradictions between nodes |
15+
| `workflow` | Process discipline checks — for keeping the tree in an operational working state (active counts, status consistency) |
1516
| `bestPractice` | Advisory guidance — signals the space may benefit from additional work |
1617

1718
## Rule Object Structure
@@ -64,29 +65,28 @@ $exists(parent) = false // true for root nodes
6465

6566
```json
6667
{
67-
"coherence": [
68+
"workflow": [
6869
{
6970
"id": "active-outcome-count",
7071
"description": "Only one outcome should be active at a time",
7172
"scope": "global",
7273
"check": "$count(nodes[resolvedType='outcome' and status='active']) <= 1"
7374
},
7475
{
75-
"id": "active-opportunity-count",
76-
"description": "Only one target opportunity should be active at a time",
77-
"scope": "global",
78-
"check": "$count(nodes[resolvedType='opportunity' and status='active']) <= 1"
76+
"id": "active-node-parent-active",
77+
"description": "An active node's parent should also be active",
78+
"check": "current.status != 'active' or $exists(parent) = false or parent.status = 'active'"
7979
}
8080
],
8181
"bestPractice": [
8282
{
8383
"id": "solution-quantity",
8484
"description": "Explore multiple candidate solutions (aim for at least three) for the target opportunity",
8585
"type": "opportunity",
86-
"check": "$count(nodes[resolvedParentTitle=$$.current.title and resolvedType='solution']) >= 3"
86+
"check": "(current.status != 'exploring' and current.status != 'active') or $count(nodes[resolvedParentTitle=$$.current.title and resolvedType='solution']) >= 3"
8787
}
8888
]
8989
}
9090
```
9191

92-
The coherence rules run against every node (no `type` filter) — each node in a space with two active outcomes will report a violation. The best-practice rule only runs against `opportunity` nodes, using `resolvedParentTitle` to count child solutions.
92+
The first workflow rule uses `scope: 'global'` — evaluated once against the whole space, producing at most one violation. The second runs per-node with no `type` filter, checking every node. The best-practice rule only runs against `opportunity` nodes where status is `exploring` or `active`, using `resolvedParentTitle` to count child solutions.

schemas/_strict.json

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"hierarchy": ["outcome", "opportunity", "solution", "assumption_test"],
1010
"allowSelfRef": ["opportunity"],
1111
"rules": {
12-
"coherence": [
12+
"workflow": [
1313
{
1414
"id": "active-outcome-count",
1515
"description": "Only one outcome should be active at a time",
@@ -21,14 +21,19 @@
2121
"description": "Only one target opportunity should be active at a time",
2222
"scope": "global",
2323
"check": "$count(nodes[resolvedType='opportunity' and status='active']) <= 1"
24+
},
25+
{
26+
"id": "active-node-parent-active",
27+
"description": "An active node's parent should also be active",
28+
"check": "current.status != 'active' or $exists(parent) = false or parent.status = 'active'"
2429
}
2530
],
2631
"bestPractice": [
2732
{
2833
"id": "solution-quantity",
2934
"description": "Explore multiple candidate solutions (aim for at least three) for the target opportunity",
3035
"type": "opportunity",
31-
"check": "$count(nodes[resolvedParentTitle=$$.current.title and resolvedType='solution']) >= 3"
36+
"check": "(current.status != 'exploring' and current.status != 'active') or $count(nodes[resolvedParentTitle=$$.current.title and resolvedType='solution']) >= 3"
3237
}
3338
]
3439
}

src/commands/validate.ts

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ import { validateHierarchy } from '../validate-hierarchy';
99
import { validateRules } from '../validate-rules';
1010

1111
interface ValidationResult {
12-
schemaValidCount: number;
13-
schemaErrorCount: number;
14-
schemaErrors: Array<{ file: string; errors: ErrorObject[] }>;
12+
validCount: number;
13+
nodeErrorCount: number;
14+
nodeErrors: Array<{ file: string; errors: ErrorObject[] }>;
1515
refErrors: Array<{ file: string; parent: string; error: string }>;
16+
configErrors: string[];
1617
ruleViolations: RuleViolation[];
1718
hierarchyViolations: HierarchyViolation[];
1819
skipped: string[];
@@ -33,10 +34,11 @@ export async function validate(path: string, options: { schema: string }): Promi
3334
}
3435

3536
const result: ValidationResult = {
36-
schemaValidCount: 0,
37-
schemaErrorCount: 0,
38-
schemaErrors: [],
37+
validCount: 0,
38+
nodeErrorCount: 0,
39+
nodeErrors: [],
3940
refErrors: [],
41+
configErrors: [],
4042
ruleViolations: [],
4143
hierarchyViolations: [],
4244
skipped,
@@ -47,10 +49,10 @@ export async function validate(path: string, options: { schema: string }): Promi
4749
const valid = validateFunc(node.schemaData);
4850

4951
if (valid) {
50-
result.schemaValidCount++;
52+
result.validCount++;
5153
} else {
52-
result.schemaErrorCount++;
53-
result.schemaErrors.push({
54+
result.nodeErrorCount++;
55+
result.nodeErrors.push({
5456
file: node.label,
5557
errors: validateFunc.errors || [],
5658
});
@@ -88,6 +90,13 @@ export async function validate(path: string, options: { schema: string }): Promi
8890

8991
// Load and execute hierarchy validation if schema defines hierarchy
9092
const metadata = loadMetadata(options.schema);
93+
94+
// Validate schema metadata configuration
95+
const badSelfRefTypes = (metadata.allowSelfRef ?? []).filter((t) => !metadata.hierarchy.includes(t));
96+
for (const t of badSelfRefTypes) {
97+
result.configErrors.push(`allowSelfRef contains "${t}" which is not in the hierarchy`);
98+
}
99+
91100
result.hierarchyViolations = validateHierarchy(nodes, metadata);
92101

93102
// Load and execute rules validation if schema defines rules
@@ -98,11 +107,12 @@ export async function validate(path: string, options: { schema: string }): Promi
98107
// Report
99108
console.log(`\n🔍 Space Validation Results`);
100109
console.log(`━`.repeat(50));
101-
console.log(`✅ Valid: ${result.schemaValidCount}`);
102-
console.log(`❌ Schema Errors: ${result.schemaErrorCount}`);
110+
console.log(`✅ Valid: ${result.validCount}`);
111+
console.log(`❌ Node Errors: ${result.nodeErrorCount}`);
103112
console.log(`🔗 Reference Errors: ${result.refErrors.length}`);
113+
console.log(`⚙️ Config Errors: ${result.configErrors.length}`);
104114
console.log(`📋 Rule Violations: ${result.ruleViolations.length}`);
105-
console.log(`🏗️ Hierarchy Violations: ${result.hierarchyViolations.length}`);
115+
console.log(`🏗️ Hierarchy Violations: ${result.hierarchyViolations.length}`);
106116
console.log(`⏭ Skipped (no frontmatter): ${result.skipped.length}`);
107117
console.log(`📄 Non-space (no type field): ${result.nonSpace.length}`);
108118

@@ -116,9 +126,9 @@ export async function validate(path: string, options: { schema: string }): Promi
116126
for (const f of result.nonSpace) console.log(` ${f}`);
117127
}
118128

119-
if (result.schemaErrors.length > 0) {
120-
console.log(`\n❌ Schema validation errors:`);
121-
result.schemaErrors.forEach(({ file, errors }) => {
129+
if (result.nodeErrors.length > 0) {
130+
console.log(`\n❌ Node errors:`);
131+
result.nodeErrors.forEach(({ file, errors }) => {
122132
console.log(`\n ${file}:`);
123133
errors.forEach((err: ErrorObject) => {
124134
console.log(` ${err.instancePath || 'root'}: ${err.message}`);
@@ -133,6 +143,13 @@ export async function validate(path: string, options: { schema: string }): Promi
133143
});
134144
}
135145

146+
if (result.configErrors.length > 0) {
147+
console.log(`\n⚙️ Config errors:`);
148+
for (const e of result.configErrors) {
149+
console.log(` ${e}`);
150+
}
151+
}
152+
136153
if (result.ruleViolations.length > 0) {
137154
console.log(`\n📋 Rule violations:`);
138155

@@ -164,8 +181,9 @@ export async function validate(path: string, options: { schema: string }): Promi
164181
console.log(`\n`);
165182

166183
if (
167-
result.schemaErrorCount > 0 ||
184+
result.nodeErrorCount > 0 ||
168185
result.refErrors.length > 0 ||
186+
result.configErrors.length > 0 ||
169187
result.ruleViolations.length > 0 ||
170188
result.hierarchyViolations.length > 0
171189
) {

src/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export interface SpaceDirectoryReadResult {
3030
}
3131

3232
/** Rule categories for organizing executable validation rules */
33-
export type RuleCategory = 'validation' | 'coherence' | 'best-practice';
33+
export type RuleCategory = 'validation' | 'coherence' | 'workflow' | 'best-practice';
3434

3535
/** A single executable rule with JSONata check expression */
3636
export interface Rule {
@@ -47,6 +47,7 @@ export interface Rule {
4747
export interface RulesMetadata {
4848
validation?: Rule[];
4949
coherence?: Rule[];
50+
workflow?: Rule[];
5051
bestPractice?: Rule[];
5152
}
5253

src/validate-rules.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export async function validateRules(nodes: SpaceNode[], rules: RulesMetadata): P
2525
const allCategories: Array<{ category: RuleCategory; rules: Rule[] }> = [
2626
{ category: 'validation', rules: rules.validation ?? [] },
2727
{ category: 'coherence', rules: rules.coherence ?? [] },
28+
{ category: 'workflow', rules: rules.workflow ?? [] },
2829
{ category: 'best-practice', rules: rules.bestPractice ?? [] },
2930
];
3031

0 commit comments

Comments
 (0)