Pull request overview

Adds a new trigger heuristic grader type to validate whether a prompt should (or should not) activate a skill, closing #80.

Changes:

• Introduces trigger grader implementation + unit tests.
• Wires trigger into grader type registries, creation factory, and JSON schemas.
• Adds end-user documentation for configuring and interpreting trigger grading output.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

func (g *triggerHeuristicGrader) scorePrompt(prompt string) (score float64, phraseScore float64, matched []string) {
	promptTokens := tokenize(prompt)
	if len(promptTokens) == 0 {
		return 0, 0, nil
	}

	seen := make(map[string]bool)
	for _, token := range promptTokens {
		if _, ok := g.keywords[token]; ok && !seen[token] {
			seen[token] = true
			matched = append(matched, token)
		}
	}

	tokenScore := float64(len(matched)) / float64(len(promptTokens))
	phraseScore = g.bestPhraseScore(prompt)
	if phraseScore > tokenScore {
		return phraseScore, phraseScore, matched
	}
	return tokenScore, phraseScore, matched
}

func (g *triggerHeuristicGrader) bestPhraseScore(prompt string) float64 {
	if len(g.triggerPhrases) == 0 {
		return 0
	}

	promptLower := strings.ToLower(prompt)
	promptTokenSet := make(map[string]struct{})
	for _, token := range tokenize(prompt) {
		promptTokenSet[token] = struct{}{}
	}

func loadTriggerHeuristicData(skillPath string) (map[string]struct{}, []string, error) {
	data, err := os.ReadFile(skillPath)
	if err != nil {
		return nil, nil, fmt.Errorf("reading SKILL.md %s: %w", skillPath, err)
	}

	var sk skill.Skill
	if err := sk.UnmarshalText(data); err != nil {
		return nil, nil, fmt.Errorf("parsing SKILL.md %s: %w", skillPath, err)
	}

    "triggerGraderConfig": {
      "type": "object",
      "required": [
        "skill_path",
        "mode"
      ],
      "additionalProperties": false,
      "properties": {
        "skill_path": {
          "type": "string",
          "minLength": 1
        },
        "mode": {
          "type": "string",
          "enum": [
            "positive",
            "negative"
          ]
        },
        "threshold": {
          "type": "number",
          "minimum": 0,
          "maximum": 1,
          "default": 0.6
        }
      }
    },

	require.True(t, result.Score >= threshold)
	require.True(t, result.Passed)

Rusty (Opus 4.6) — ⚠️ Blocked: Merge Conflict

The trigger heuristic grader implementation is solid architecturally — clean grader pattern, constructor validation, 4 test functions, \docs/graders/trigger.md, schema updates. I want to approve this.

Blocker: Merge conflict. GitHub reports \mergeable: CONFLICTING. Only 1 CI check (CLA) ran — the full Go CI matrix didn't execute.

Action needed:

1. Rebase onto main to resolve the merge conflict (likely in \internal/models/outcome.go\ where fields overlap with PR #91)
2. Verify all 7 CI checks pass
3. Push the rebased branch

Once CI is green, I'll approve immediately.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Code Review: PR #90 - feat: Add trigger heuristic grader

What Looks Good

• Follows existing grader patterns
• Clean mode separation with good tests
• Smart keyword extraction strips DO NOT USE FOR content
• Dual scoring approach with good coverage
• Thorough constructor validation and docs included

Suggestions (non-blocking)

1. Stop words include trigger verbs (use, when, help). Consider excluding from stop words.

Summary

Overall Assessment: Approve

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Code Review: PR #90 — feat: Add trigger heuristic grader

✅ What Looks Good

• Follows existing grader patterns — factory registration, measureTime wrapper, Grader interface, consistent details map
• Clean scoring design — max(tokenScore, phraseScore) gives two complementary signals
• Proper anti-trigger exclusion — strips DO NOT USE FOR: before keyword extraction
• Thoughtful tokenizer — stop words, min length, unicode-aware
• Constructor validation — clear error messages for mode, threshold, skill_path
• Schema, docs, and suggest registration all updated consistently

Findings Summary

Overall Assessment: Approve — clean implementation. Low findings are minor observations only.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

	if resp == nil {
		t.Fatal("expected response, got nil")
		return
	}

	if resp == nil {
		t.Fatal("expected response, got nil")
		return
	}

	if resp == nil {
		t.Fatal("expected response, got nil")
		return
	}

	if resp == nil {
		t.Fatal("expected response, got nil")
		return
	}

	if resp == nil {
		t.Fatal("expected response, got nil")
		return
	}

	if resp == nil {
		t.Fatal("expected response, got nil")
		return
	}

	if resp == nil {
		t.Fatal("expected response, got nil")
		return
	}

Re: return after t.Fatal — these are intentional. staticcheck SA5011 doesn't recognize t.Fatal as a terminating call and flags the subsequent resp.Error access as a possible nil pointer dereference. The return is the standard workaround to satisfy the linter.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

You can also share your feedback on Copilot code review. Take the survey.

Re-Review: PR #90 — feat: Add trigger heuristic grader

All previous review feedback has been properly addressed:

✅ Stop words updated — Removed 'when', 'use', 'help' to preserve trigger verbs
✅ Double tokenization eliminated — �estPhraseScore now receives pre-computed tokens
✅ Nil context handling added — TestTriggerHeuristicGrader_NilGradingContext covers both nil context and nil TestCase
✅ Empty prompt guard added — Returns clear feedback when prompt is empty
✅ Unique token normalization — Score calculation divides by unique token count to prevent dilution
✅ Comprehensive test coverage — Including duplicate token test verifying normalization works correctly

The implementation is clean, well-tested, and follows existing patterns. All tests pass.

Approval: ✅ Ready to merge