feat: add filtering support for spaces

[mindsocket]

Summary

• Adds a --filter option to the show command for filtering space nodes by expression
• Implements a SELECT-style filter expression parser supporting node types, INCLUDE for subtree expansion, and ALL
• New filter/ module: parse-expression, filter-nodes, augment-nodes, expand-include
• Updates docs/concepts.md and README with filtering concepts and usage

Test plan

• bun test tests/filter/ — all filter unit tests pass
• ost-tools show <space> --filter "opportunity" returns only opportunity nodes
• ost-tools show <space> --filter "INCLUDE opportunity" returns opportunities with their full subtrees

[claude]

Claude finished @mindsocket's task in 1m 27s —— View job

---

Code Review Complete

Reviewing PR #66: feat: add filtering support for spaces

Summary

This PR adds comprehensive filtering capabilities to the show command via a new --filter option. The implementation is well-designed with clear separation of concerns, excellent test coverage (69 passing tests), and thorough documentation updates.

---

Overall Assessment ✅

Quality: High - Clean architecture, good type safety, comprehensive tests
Test Coverage: Excellent - 69 tests across 4 files, all passing
Documentation: Thorough - README, concepts.md, and skills documentation updated
Ready to Merge: Yes, with minor suggestions below

---

Detailed Findings

✅ Strengths

1. Well-designed DSL - The SELECT/WHERE syntax is intuitive and follows SQL-like conventions:

   • WHERE {jsonata} for filtering
   • SELECT {spec} WHERE {jsonata} for filter + expand
   • SELECT {spec} for expansion only
   • Bare JSONata as convenience shorthand

2. Comprehensive test coverage - Tests cover:

   • All parser edge cases (empty clauses, case-insensitivity, malformed input)
   • Graph traversal (ancestors, descendants, siblings, relationships)
   • Diamond DAG deduplication
   • Cycle detection with selfRef
   • Type filtering and edge metadata preservation

3. Good code organization - Clear separation of concerns:

   • parse-expression.ts - DSL parsing
   • filter-nodes.ts - Main orchestration
   • augment-nodes.ts - Graph traversal/pre-computation
   • expand-include.ts - Result expansion

4. Type safety - Strong TypeScript usage with discriminated unions for directives

5. Performance considerations:

   • Expression caching in filterNodes (src/filter/filter-nodes.ts:7)
   • BFS algorithms with O(V+E) complexity
   • Pre-computed augmented representations

---

⚠️ Minor Issues & Suggestions

1. Unbounded expression cache (Low Priority)

• Location: src/filter/filter-nodes.ts:7
• Issue: The expressionCache Map could grow indefinitely with unique expressions
• Impact: Minimal for CLI usage, but worth noting
• Suggestion: Consider cache size limits or LRU eviction if this becomes a concern

2. Error message could be clearer

• Location: src/filter/parse-expression.ts:44-47
• Issue: Error "SELECT clause must not be empty" may confuse users who wrote SELECT WHERE expr
• Current: When detecting WHERE at end of include, it throws the empty SELECT error
• Suggestion: Could provide a more specific message like "SELECT clause is missing content (found WHERE without content before it)"

3. No explicit "ALL" keyword

• Location: Test plan in PR description
• Issue: Test plan mentions ost-tools show <space> --filter "ALL" but this isn't explicitly supported
• Current: Users would use bare SELECT or SELECT siblings, ancestors etc.
• Suggestion: Either document that bare SELECT achieves "ALL" behavior, or implement explicit ALL keyword

4. Documentation: Clarify matched nodes behavior

• Location: SELECT clause expansion
• Issue: When filtering with SELECT ancestors(goal) WHERE resolvedType='solution', the result includes both matched solutions AND their goal ancestors
• Current: This is correct behavior, but could be clearer in docs
• Suggestion: Add a note in README that "matched nodes are always included in the result"

5. Siblings directive may include duplicates

• Location: src/filter/expand-include.ts:184-195
• Issue: If a node has multiple parents, siblings from each parent are added (deduplicated per parent, but not across parents)
• Current: This is by design due to seen set, but worth documenting
• Impact: Low - the seen set already prevents true duplicates

---

✅ Code Quality Highlights

Good Practices:

• BFS traversal with cycle detection (src/filter/augment-nodes.ts:86-104)
• Edge metadata preservation (_field, _source, _selfRef)
• Diamond DAG deduplication handling
• Case-insensitive keyword parsing
• Comprehensive error messages for malformed input

Type Safety:

// src/filter/expand-include.ts:8-38
export type AncestorsDirective = { kind: 'ancestors'; typeFilter?: string };
export type DescendantsDirective = { kind: 'descendants'; typeFilter?: string };
export type SiblingsDirective = { kind: 'siblings' };
export type RelationshipsDirective = { kind: 'relationships'; childType?: string; ... };
export type IncludeDirective = AncestorsDirective | DescendantsDirective | SiblingsDirective | RelationshipsDirective;

---

✅ Security Considerations

• JSONata expression evaluation is sandboxed within the jsonata library
• No code injection vulnerabilities identified
• Input validation is thorough (empty checks, malformed input detection)
• No external command execution

---

✅ Documentation Coverage

All required locations updated per CLAUDE.md:

• ✅ README.md - New "Filter expressions" section with comprehensive examples
• ✅ docs/concepts.md - Filter concepts added (lines 100+)
• ✅ skills/ost-tools/references/commands.md - show command updated with --filter examples
• ✅ src/config.ts - Views type and schema added

---

Test Results

bun test tests/filter/
69 pass, 0 fail, 111 expect() calls

All tests pass, including edge cases for:

• Empty expressions and clauses
• Case-insensitive keyword parsing
• Diamond DAG deduplication
• Self-referential cycles
• Relationship edge filtering
• Multiple directive combinations

---

Recommendations

1. Before merge: Consider adding one more test for the "ALL" behavior mentioned in test plan (or clarify test plan)
2. Future enhancement: Add explicit ALL keyword for clarity
3. Future enhancement: Consider LRU cache for expressionCache if many unique expressions are expected

---

Conclusion

This is well-crafted feature implementation with excellent test coverage and documentation. The code is clean, type-safe, and follows project conventions. The minor issues identified are suggestions for future enhancements rather than blockers.

Recommendation: Approve with optional follow-ups

--- | Branch: feat/filtering

[claude]

Claude finished @mindsocket's task in 1m 44s —— View job

---

Code Review Complete

Reviewing PR #66: feat: add filtering support for spaces

Summary

This PR adds comprehensive filtering capabilities to the show command via a new --filter option. The implementation is well-designed with clear separation of concerns, excellent test coverage (69 passing tests), and thorough documentation updates.

---

Overall Assessment ✅

Quality: High - Clean architecture, good type safety, comprehensive tests
Test Coverage: Excellent - 69 tests across 4 files, all passing
Documentation: Thorough - README, concepts.md, and skills documentation updated
Ready to Merge: Yes, with minor suggestions below

---

Detailed Findings

✅ Strengths

1. Well-designed DSL - The SELECT/WHERE syntax is intuitive and follows SQL-like conventions:

   • WHERE {jsonata} for filtering
   • SELECT {spec} WHERE {jsonata} for filter + expand
   • SELECT {spec} for expansion only
   • Bare JSONata as convenience shorthand

2. Comprehensive test coverage - Tests cover:

   • All parser edge cases (empty clauses, case-insensitivity, malformed input)
   • Graph traversal (ancestors, descendants, siblings, relationships)
   • Diamond DAG deduplication
   • Cycle detection with selfRef
   • Type filtering and edge metadata preservation

3. Good code organization - Clear separation of concerns:

   • parse-expression.ts - DSL parsing
   • filter-nodes.ts - Main orchestration
   • augment-nodes.ts - Graph traversal/pre-computation
   • expand-include.ts - Result expansion

4. Type safety - Strong TypeScript usage with discriminated unions for directives

5. Performance considerations:

   • Expression caching in filterNodes (filter-nodes.ts:7)
   • BFS algorithms with O(V+E) complexity
   • Pre-computed augmented representations

6. Good error handling improvement - Latest commit (f20976d) correctly removed try-catch that was suppressing evaluation errors, making them hard failures as they should be.

---

⚠️ Minor Issues & Suggestions

1. Unbounded expression cache (Low Priority)

• Location: filter-nodes.ts:7
• Issue: The expressionCache Map could grow indefinitely with unique expressions
• Impact: Minimal for CLI usage, but worth noting
• Suggestion: Consider cache size limits or LRU eviction if this becomes a concern

2. Error message could be clearer

• Location: parse-expression.ts:44-47
• Issue: Error "SELECT clause must not be empty" may confuse users who wrote SELECT WHERE expr
• Current: When detecting WHERE at end of include, it throws the empty SELECT error
• Suggestion: Could provide a more specific message like "SELECT clause is missing content (found WHERE without content before it)"

3. No explicit "ALL" keyword

• Location: Test plan in PR description
• Issue: Test plan mentions ost-tools show <space> --filter "ALL" but this isn't explicitly supported
• Current: Users would use bare SELECT or SELECT siblings, ancestors etc.
• Suggestion: Either document that bare SELECT achieves "ALL" behavior, or implement explicit ALL keyword

4. Documentation: Clarify matched nodes behavior

• Location: SELECT clause expansion
• Issue: When filtering with SELECT ancestors(goal) WHERE resolvedType='solution', the result includes both matched solutions AND their goal ancestors
• Current: This is correct behavior, but could be clearer in docs
• Suggestion: Add a note in README that "matched nodes are always included in the result"

---

✅ Code Quality Highlights

Good Practices:

• BFS traversal with cycle detection (augment-nodes.ts:84-104)
• Edge metadata preservation (_field, _source, _selfRef)
• Diamond DAG deduplication handling
• Case-insensitive keyword parsing
• Comprehensive error messages for malformed input

Type Safety:

// expand-include.ts:8-38 - Discriminated unions for directives
export type AncestorsDirective = { kind: 'ancestors'; typeFilter?: string };
export type DescendantsDirective = { kind: 'descendants'; typeFilter?: string };
export type SiblingsDirective = { kind: 'siblings' };
export type RelationshipsDirective = { kind: 'relationships'; childType?: string; ... };

---

✅ Security Considerations

• JSONata expression evaluation is sandboxed within the jsonata library
• No code injection vulnerabilities identified
• Input validation is thorough (empty checks, malformed input detection)
• No external command execution

---

✅ Documentation Coverage

All required locations updated per CLAUDE.md:

• ✅ README.md - New "Filter expressions" section with comprehensive examples
• ✅ docs/concepts.md - Filter concepts added
• ✅ skills/ost-tools/references/commands.md - show command updated with --filter examples
• ✅ src/config.ts - Views type and schema added

---

Test Results

bun test tests/filter/
69 pass, 0 fail, 111 expect() calls

All tests pass, including edge cases for:

• Empty expressions and clauses
• Case-insensitive keyword parsing
• Diamond DAG deduplication
• Self-referential cycles
• Relationship edge filtering
• Multiple directive combinations

---

Recommendations

1. Before merge: Consider clarifying in test plan or README that bare SELECT achieves "ALL" behavior
2. Future enhancement: Add explicit ALL keyword for clarity
3. Future enhancement: Consider LRU cache for expressionCache if many unique expressions are expected

---

Conclusion

This is well-crafted feature implementation with excellent test coverage and documentation. The code is clean, type-safe, and follows project conventions. The latest commit properly addressed error handling by making evaluation errors hard failures. The minor issues identified are suggestions for future enhancements rather than blockers.

Recommendation: Approve with optional follow-ups

---

| Branch: feat/filtering

[claude]

Claude finished @mindsocket's task in 1m 39s —— View job

---

Code Review Complete

Reviewing PR #66: feat: add filtering support for spaces

Summary

This PR adds comprehensive filtering capabilities to the show command via a new --filter option. The implementation is well-designed with clear separation of concerns, excellent test coverage (69 passing tests), and thorough documentation updates.

---

Overall Assessment ✅

Quality: High - Clean architecture, good type safety, comprehensive tests
Test Coverage: Excellent - 69 tests across 4 files, all passing
Documentation: Thorough - README, concepts.md, and skills documentation updated
Ready to Merge: ✅ Yes

---

Latest Changes Analysis

The most recent commit (8cbae0d) addresses a biome linting rule (useLiteralKeys) by converting bracket notation to dot notation in test files:

// Before
expect(result['status']).toBe('active');
expect((node as Record<string, unknown>)['ancestors']).toBeUndefined();

// After
expect(result.status).toBe('active');
expect(((node as Record<string, unknown>).ancestors)).toBeUndefined();

All 69 tests pass successfully with these changes.

---

Detailed Findings

✅ Strengths

1. Well-designed DSL - The SELECT/WHERE syntax is intuitive and follows SQL-like conventions:

   • WHERE {jsonata} for filtering
   • SELECT {spec} WHERE {jsonata} for filter + expand
   • SELECT {spec} for expansion only
   • Bare JSONata as convenience shorthand

2. Comprehensive test coverage - Tests cover:

   • All parser edge cases (empty clauses, case-insensitivity, malformed input)
   • Graph traversal (ancestors, descendants, siblings, relationships)
   • Diamond DAG deduplication
   • Cycle detection with selfRef
   • Type filtering and edge metadata preservation

3. Good code organization - Clear separation of concerns:

   • parse-expression.ts - DSL parsing
   • filter-nodes.ts - Main orchestration
   • augment-nodes.ts - Graph traversal/pre-computation
   • expand-include.ts - Result expansion

4. Type safety - Strong TypeScript usage with discriminated unions for directives

5. Performance considerations:

   • Expression caching in filterNodes (filter-nodes.ts:7)
   • BFS algorithms with O(V+E) complexity
   • Pre-computed augmented representations

6. Good error handling - Commit f20976d correctly removed try-catch that was suppressing evaluation errors, making them hard failures as they should be.

---

⚠️ Minor Observations

1. Unbounded expression cache (Low Priority)

• Location: filter-nodes.ts:7
• Issue: The expressionCache Map could grow indefinitely with unique expressions
• Impact: Minimal for CLI usage, but worth noting
• Suggestion: Consider cache size limits or LRU eviction if this becomes a concern

2. Unnecessary parentheses in test (Cosmetic)

• Location: filter-nodes.test.ts:102
• Current: expect(((node as Record<string, unknown>).ancestors)).toBeUndefined();
• Note: The double parentheses are likely from biome auto-fix and don't affect functionality

3. No explicit "ALL" keyword

• Location: Test plan in PR description
• Issue: Test plan mentions ost-tools show <space> --filter "ALL" but this isn't explicitly supported
• Current: Users would use bare SELECT or SELECT siblings, ancestors etc.
• Suggestion: Either document that bare SELECT achieves "ALL" behavior, or implement explicit ALL keyword

4. Documentation: Clarify matched nodes behavior

• Location: SELECT clause expansion
• Issue: When filtering with SELECT ancestors(goal) WHERE resolvedType='solution', the result includes both matched solutions AND their goal ancestors
• Current: This is correct behavior, but could be clearer in docs
• Suggestion: Add a note in README that "matched nodes are always included in the result"

---

✅ Code Quality Highlights

Good Practices:

• BFS traversal with cycle detection (augment-nodes.ts:84-104)
• Edge metadata preservation (_field, _source, _selfRef)
• Diamond DAG deduplication handling
• Case-insensitive keyword parsing
• Comprehensive error messages for malformed input

Type Safety:

// expand-include.ts:8-38 - Discriminated unions for directives
export type AncestorsDirective = { kind: 'ancestors'; typeFilter?: string };
export type DescendantsDirective = { kind: 'descendants'; typeFilter?: string };
export type SiblingsDirective = { kind: 'siblings' };
export type RelationshipsDirective = { kind: 'relationships'; childType?: string; ... };

---

✅ Security Considerations

• JSONata expression evaluation is sandboxed within the jsonata library
• No code injection vulnerabilities identified
• Input validation is thorough (empty checks, malformed input detection)
• No external command execution

---

✅ Documentation Coverage

All required locations updated per CLAUDE.md:

• ✅ README.md - New "Filter expressions" section with comprehensive examples
• ✅ docs/concepts.md - Filter concepts added
• ✅ skills/ost-tools/references/commands.md - show command updated with --filter examples
• ✅ src/config.ts - Views type and schema added

---

Test Results

bun test tests/filter/
69 pass, 0 fail, 111 expect() calls

All tests pass, including edge cases for:

• Empty expressions and clauses
• Case-insensitive keyword parsing
• Diamond DAG deduplication
• Self-referential cycles
• Relationship edge filtering
• Multiple directive combinations

---

Recommendations

1. Before merge: Consider clarifying in test plan or README that bare SELECT achieves "ALL" behavior
2. Future enhancement: Add explicit ALL keyword for clarity
3. Future enhancement: Consider LRU cache for expressionCache if many unique expressions are expected

---

Conclusion

This is a well-crafted feature implementation with excellent test coverage and documentation. The code is clean, type-safe, and follows project conventions. The latest commit properly addresses biome linting requirements. All minor issues identified are suggestions for future enhancements rather than blockers.

Recommendation: ✅ Approve - Ready to merge

---

| Branch: feat/filtering