/claim #837

Enhance Coverage Command for Detailed Action/Permission Conditions (#837)

Description

This PR implements comprehensive coverage tracking for permission rules, specifically detecting when parts of conditions are skipped due to short-circuit evaluation (e.g., when A OR B evaluates A as true, B is never executed).

Problem

Previously, the coverage command would mark a permission as “covered” if any part of the condition was asserted, even if other parts were skipped due to short-circuit logic. This led to incomplete coverage assessments.

Example from issue:

permission view = system.view or ((is_public or (is_partner and partner) or (viewer or company.maintain or organization.maintain or team.view)) not denied)

Asserting only system.view would mark the entire permission as covered, even though other components were never tested.

Solution

Implemented a comprehensive coverage tracking system that:

1. AST Discovery: Registers all logic nodes (AND, OR, leaf nodes) with their source positions (Line:Column)
2. Coverage Registry: Tracks visit counts for each node during evaluation
3. Evaluator Instrumentation: Wraps all evaluation paths to track node visits
4. Short-Circuit Detection: Identifies nodes that remain unvisited due to early returns
5. Detailed Reporting: Reports uncovered nodes with exact source positions

Implementation Details

New Components

• internal/coverage/registry.go: Thread-safe registry for tracking node visits with SourceInfo (Line:Column)
• internal/coverage/discovery.go: AST walker that discovers and registers all logic nodes during schema parsing
• internal/engines/coverage_test.go: Comprehensive test for short-circuit detection

Modified Components

• internal/engines/check.go: Added trace() wrapper to instrument evaluation paths and track visits
• pkg/development/development.go: Integrated logic coverage into coverage command output
• pkg/development/coverage/coverage.go: Extended coverage info with logic node tracking
• pkg/cmd/coverage.go: Updated to display logic coverage information
• proto/base/v1/base.proto: Added PositionInfo message for source position tracking
• pkg/dsl/compiler/compiler.go: Enhanced to populate PositionInfo during compilation

Features

✅ Source Position Tracking: Every node includes Line:Column information from source
✅ Unique Node IDs: Deterministic path-based IDs (e.g., repository#edit.0, repository#edit.1)
✅ Short-Circuit Detection: Correctly identifies skipped nodes in:

• UNION (OR) operations: When first operand succeeds, remaining operands are marked uncovered
• INTERSECTION (AND) operations: When first operand fails, remaining operands are marked uncovered
  ✅ Thread-Safe: Registry uses sync.RWMutex for concurrent access
  ✅ Comprehensive Reporting: Uncovered nodes reported with exact source positions
  ✅ Backward Compatible: No breaking changes to existing coverage functionality

Testing

• ✅ TestCheckEngineCoverage: Verifies short-circuit detection for OR operations
• ✅ TestRegistry: Validates registry functionality and thread safety
• ✅ TestDiscover: Confirms AST discovery correctly registers all logic nodes

Test Case:

permission edit = owner or admin
// When owner=true, admin (path: repository#edit.1) correctly identified as uncovered

All tests pass successfully.

Example Output

The coverage command now reports:

• Total logic coverage percentage
• Per-entity logic coverage
• Uncovered logic nodes with Line:Column positions

Example:

Logic Coverage: 50%
Uncovered Logic Nodes:
  - repository#edit.1 (Line: 26, Column: 25) [OR]
    Description: Second operand of OR expression was skipped due to short-circuit

Technical Approach

1. AST Updates: Leveraged existing PositionInfo in tokens to track source positions
2. Discovery Phase: Walk AST during schema parsing to register all logic nodes
3. Instrumentation: Wrap evaluation functions with tracking to record visits
4. Short-Circuit Handling: Context cancellation prevents execution of skipped branches
5. Reporting: Query registry for nodes with VisitCount == 0 to identify gaps

Performance Considerations

• Minimal overhead: Single map lookup per node during evaluation
• Efficient path-based lookup: O(1) map access
• Thread-safe operations: Safe for concurrent evaluation scenarios

Breaking Changes

None. This is a backward-compatible enhancement.

Checklist

• Code follows the project’s style guidelines
• Tests added/updated and passing
• Documentation updated (COVERAGE_READINESS.md)
• All existing tests pass
• No breaking changes
• Thread-safe implementation
• Source positions correctly tracked

Related Issues

Closes #837

Additional Notes

See COVERAGE_READINESS.md for detailed implementation assessment and verification. The implementation correctly detects when parts of permission rules are skipped due to short-circuit evaluation, providing detailed coverage information with exact source positions for uncovered nodes.

Summary by CodeRabbit

• New Features

  • Coverage tracking for permission logic with per-node source positions (line:column), deterministic node IDs, and CLI display of coverage percent and uncovered nodes
  • New coverage metadata field added to permission check requests (coverage_path)

• Behavioral

  • Accurate short-circuit detection so skipped branches are reported as uncovered

• Tests

  • New unit and integration tests validating discovery, registry tracking, short-circuit coverage, and end-to-end reporting