feat(web-audit): add targeted failOnIssueCodes publish gate

[PrzemyslawKlys]

Summary

• add FailOnIssueCodes to web audit options for targeted publish blocking
• wire option through web audit, doctor, and pipeline runner (including aliases)
• enrich WebAuditIssue with Code and Hint metadata
• add schema support and documentation updates
• add tests covering matching and non-matching issue-code gate behavior

Why

failOnCategories is too coarse for CI quality gates. This enables strict blocking on specific regressions (for example media-img-dimensions, heading-order, head-render-blocking) without failing whole categories.

Validation

• dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj -c Release --filter "FullyQualifiedName~WebSiteAuditOptimizeBuildTests|FullyQualifiedName~WebSiteAuditFileBudgetTests"

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[claude]

PR Review: feat(web-audit): add targeted failOnIssueCodes publish gate

Summary

This PR adds a fine-grained quality gate mechanism (failOnIssueCodes) to the web audit system, allowing CI pipelines to fail on specific issue codes rather than broad categories. The implementation is well-structured and follows the codebase patterns consistently.

---

✅ Strengths

1. Well-Motivated Feature

The PR addresses a real pain point: failOnCategories is too coarse for surgical quality gates. The ability to block on specific issues like media-img-dimensions or heading-order without failing entire categories is valuable for incremental quality improvements.

2. Consistent Implementation

• The feature is properly threaded through all entry points: CLI (--fail-issue-code), pipeline JSON (failOnIssueCodes), and the doctor command
• Multiple aliases are supported (failIssueCodes, failOnIssues, failIssues) matching existing patterns
• Schema updates included for JSON validation

3. Robust Pattern Matching

The implementation leverages the existing WebSuppressionMatcher which supports:

• Exact code matching (PFAUDIT.MEDIA.MEDIA-IMG-DIMENSIONS)
• Normalized hint tokens (media-img-dimensions)
• Wildcards (*, ?)
• Regex patterns (re:...)
• Substring matching

This provides excellent flexibility for users.

4. Good Test Coverage

Two focused unit tests cover the critical paths:

• Audit_FailOnIssueCodes_TriggersGateForMatchingIssueHint: Verifies the gate fails when patterns match
• Audit_FailOnIssueCodes_DoesNotFailWhenPatternDoesNotMatch: Verifies the gate doesn't trigger on non-matching patterns

5. Documentation

• Updated PowerForge.Web.Pipeline.md with usage examples
• Updated PowerForge.Web.QualityGates.md with context and guidance
• XML doc comments on FailOnIssueCodes property are comprehensive

---

🔍 Code Quality Observations

Architecture

The implementation follows the existing audit gate pattern (similar to FailOnCategories):

1. Patterns normalized via WebSuppressionMatcher.NormalizePatterns
2. Issues enriched with Code and Hint metadata during creation
3. Gate check performed after all issues collected but before result finalized
4. Excludes "gate" category issues from the check to prevent recursion

Key Implementation Details

Issue Code Generation (WebSiteAuditor.AssetsAndRendered.cs:237-252):

private static string BuildIssueRuleCode(string category, string? hint)
{
    var categoryCode = BuildIssueCategoryCode(category);
    var normalizedHint = NormalizeIssueToken(hint);
    if (string.IsNullOrWhiteSpace(normalizedHint))
        return categoryCode;
    return categoryCode + "." + normalizedHint.ToUpperInvariant();
}

This creates hierarchical codes like PFAUDIT.MEDIA.MEDIA-IMG-DIMENSIONS.

Pattern Matching (WebSiteAuditor.AssetsAndRendered.cs:254-274):
The MatchesFailIssuePatterns method checks multiple representations:

• Full rule code
• Category code
• Normalized hint
• Issue key
• Full descriptor string

This multi-pronged approach ensures users can match issues in the most intuitive way.

---

🐛 Potential Issues

1. Minor: Inconsistent Exclusion Logic

Location: WebSiteAuditor.cs:627-629

var issueHits = issues.Count(issue =>
    !issue.Category.Equals("gate", StringComparison.OrdinalIgnoreCase) &&
    MatchesFailIssuePatterns(issue, failIssuePatterns));

Issue: The FailOnCategories check (lines 616-622) does NOT exclude "gate" category issues, but FailOnIssueCodes does.

Impact: Low - "gate" issues shouldn't match user patterns in practice, but the inconsistency is worth noting.

Recommendation: Either add the same exclusion to FailOnCategories for consistency, or document why FailOnIssueCodes needs it.

---

2. Minor: Test Coverage Gap

The tests validate basic matching behavior, but don't cover:

• Wildcard patterns (media-*)
• Regex patterns (re:media.*dimensions)
• Full code matching (PFAUDIT.MEDIA.MEDIA-IMG-DIMENSIONS)
• Edge cases like empty/whitespace patterns

Recommendation: Consider adding a parameterized test covering different pattern types:

[Theory]
[InlineData("media-img-dimensions")]      // hint match
[InlineData("PFAUDIT.MEDIA.MEDIA-IMG-DIMENSIONS")] // code match
[InlineData("media-*")]                   // wildcard
[InlineData("re:media.*dim")]             // regex
public void Audit_FailOnIssueCodes_SupportsVariousPatterns(string pattern)
{
    // ...
}

---

3. Minor: Documentation Could Be Clearer on Pattern Priority

The XML docs and user documentation mention all pattern types but don't explain the matching order/priority:

1. Exact code match
2. Exact category match
3. Hint match
4. Key match
5. Descriptor substring match

Recommendation: Add a note in WebAuditOptions.cs explaining that patterns are OR'd together and which representations are checked.

---

🔒 Security Considerations

✅ No Issues Detected

• Regex matching has proper timeout protection (1 second in WebSuppressionMatcher)
• Invalid regex patterns are caught and ignored (try/catch)
• No injection vectors identified
• Pattern matching operates on pre-sanitized issue metadata

---

⚡ Performance Considerations

✅ Acceptable Performance

The gate check is O(n*m) where n = issue count and m = pattern count:

var issueHits = issues.Count(issue =>
    !issue.Category.Equals("gate", StringComparison.OrdinalIgnoreCase) &&
    MatchesFailIssuePatterns(issue, failIssuePatterns));

For each issue, MatchesFailIssuePatterns checks 5 representations against all patterns.

Typical Scale:

• Issues: 10-1000
• Patterns: 1-20

This should complete in milliseconds. Regex timeout protection prevents runaway patterns.

Optimization Opportunity (Future): If pattern counts grow large, could pre-compile regex patterns and cache wildcard-to-regex conversions.

---

📝 Best Practices

✅ Follows Codebase Conventions

• Thin CLI handlers that delegate to core services ✓
• Typed models with XML docs ✓
• Consistent alias support across entry points ✓
• Schema updates for pipeline JSON ✓

✅ No Breaking Changes

• New optional property with safe default (Array.Empty<string>())
• Additive API only

✅ Reasonable Code Reuse

• Leverages existing WebSuppressionMatcher infrastructure
• Follows FailOnCategories pattern

---

🧪 Testing Recommendations

Current Coverage: Basic happy path + negative case
Suggested Additions:

1. Pattern variety test (as mentioned above)
2. Edge cases:
   • Empty pattern array (should not fail)
   • Patterns with only whitespace (should be ignored)
   • Multiple patterns where only one matches
3. Integration test showing failOnCategories and failOnIssueCodes can work together
4. Baseline interaction: Verify failOnIssueCodes respects baseline suppression via FailOnNewIssues

---

📋 Minor Nits

1. Line 627-628: The gate category exclusion could be extracted to a named constant or helper method for clarity:

   private static bool IsGateIssue(WebAuditIssue issue) =>
       issue.Category.Equals("gate", StringComparison.OrdinalIgnoreCase);

2. Documentation: The PR body mentions running specific tests, but doesn't show the test results. Consider including test output in future PRs.

3. Schema file: The JSON schema adds 4 aliases for the same property. While this matches existing patterns (e.g., failOnCategories/failCategories), it might be worth consolidating in a future refactor to reduce API surface.

---

✅ Final Verdict

Recommendation: APPROVE with minor suggestions

This is a solid, well-implemented feature that fills a real need. The code quality is high, follows established patterns, and includes appropriate test coverage. The suggestions above are minor improvements rather than blockers.

What I Like Most:

• Clear motivation and use case
• Consistent implementation across all entry points
• Reuses proven pattern matching infrastructure
• Good documentation updates

What Could Be Better (non-blocking):

• Slightly more comprehensive test coverage
• Minor consistency fix for gate exclusion logic
• Clearer pattern matching priority documentation

Great work! 🎉

---

📚 References

Key files reviewed:

• PowerForge.Web/Services/WebSiteAuditor.cs:54-95, 625-636
• PowerForge.Web/Services/WebSiteAuditor.AssetsAndRendered.cs:211-274
• PowerForge.Web/Models/WebAuditOptions.cs:162-168
• PowerForge.Web/Models/WebAuditIssue.cs:10-13
• PowerForge.Tests/WebSiteAuditOptimizeBuildTests.Part3b.cs:48-128
• PowerForge.Web/Services/WebSuppressionMatcher.cs