Skip to content

Conversation

@Skyline-23
Copy link

@Skyline-23 Skyline-23 commented Jan 4, 2026

feat(hooks): add comment-checker gating to skip unnecessary CLI calls

  • Add shouldRunCommentChecker() to detect comment markers in content
  • Skip CLI execution when no comment-bearing content detected
  • Reduce blocking latency for edits without comments

Summary

  • Add content-based gating to skip comment-checker CLI when no comment markers present
  • Reduce unnecessary blocking latency for edits/writes that don't contain comments

Changes

  • src/hooks/comment-checker/index.ts: Added shouldRunCommentChecker() with COMMENT_MARKERS detection, integrated gating into processWithCli() flow
  • src/hooks/comment-checker/index.test.ts: Added unit tests for gating behavior

Testing

bun run typecheck
bun test src/hooks/comment-checker/index.test.ts

Related Issues


Summary by cubic

Run the comment-checker only when content or edits contain comment markers, skipping the CLI to reduce latency for non-comment edits.

  • New Features

    • Added shouldRunCommentChecker to detect //, /* */, #, , ''', """, and --.
    • Integrated gating before invoking the CLI to return early when no markers are found.
    • Added unit tests for content, edits, and no-marker cases.
  • Bug Fixes

    • Log errors when parsing post-tool-use hook output fails.

Written for commit 9e77eed. Summary will update on new commits.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2026

All contributors have signed the CLA. Thank you! ✅
Posted by the CLA Assistant Lite bot.

@Skyline-23
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@Skyline-23 Skyline-23 closed this Jan 4, 2026
@Skyline-23 Skyline-23 reopened this Jan 4, 2026
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed04e35b2b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@greptile-apps
Copy link

greptile-apps bot commented Jan 4, 2026

Greptile Summary

Added content-based gating to skip comment-checker CLI execution when no comment markers are detected in file operations (Write/Edit/MultiEdit tools), reducing unnecessary blocking latency.

Key Changes:

  • Introduced COMMENT_MARKERS array with 8 common patterns: //, /* */, #, <!-- -->, ''', """
  • Added hasCommentMarkers() helper to detect marker presence in strings
  • Added shouldRunCommentChecker() to check all relevant fields (content, oldString, newString, edits array)
  • Integrated gating check in tool.execute.after before CLI invocation
  • Added unit tests covering content, edits, and no-marker scenarios

Impact:
This optimization skips the blocking CLI call for file operations that contain no comments, improving responsiveness for pure code changes while maintaining full checking for comment-bearing content. The gating is conservative (may have false positives for markers in strings/URLs) which is acceptable as it prioritizes safety over performance.

Confidence Score: 5/5

  • This PR is safe to merge with no risk - clean performance optimization with proper tests
  • The implementation is straightforward, well-tested, and conservative by design. The gating logic correctly checks all relevant fields, tests verify expected behavior, and the conservative approach (false positives are acceptable) ensures no comments are missed. No breaking changes, no security concerns, and follows project conventions (TDD with BDD-style comments).
  • No files require special attention

Important Files Changed

Filename Overview
src/hooks/comment-checker/index.ts Added content-based gating with shouldRunCommentChecker() to skip CLI when no comment markers detected, reducing latency for non-comment edits
src/hooks/comment-checker/index.test.ts Added unit tests for gating logic covering content, edits, and no-marker scenarios

Sequence Diagram

sequenceDiagram
    participant Agent
    participant Hook as comment-checker hook
    participant Gate as shouldRunCommentChecker()
    participant CLI as comment-checker CLI
    
    Agent->>Hook: tool.execute.before (Write/Edit/MultiEdit)
    Note over Hook: Store pendingCall with<br/>content/edits in Map
    
    Agent->>Agent: Execute tool (write file/edit content)
    
    Agent->>Hook: tool.execute.after
    Hook->>Hook: Retrieve pendingCall from Map
    Hook->>Hook: Check if CLI available
    
    alt CLI not available
        Hook-->>Agent: Skip (return early)
    else CLI available
        Hook->>Gate: shouldRunCommentChecker(pendingCall)
        Gate->>Gate: Check content for markers<br/>(// /* # <!-- ''' """)
        Gate->>Gate: Check oldString/newString for markers
        Gate->>Gate: Check edits array for markers
        
        alt No comment markers found
            Gate-->>Hook: false
            Hook-->>Agent: Skip CLI call (return early)
            Note over Hook,CLI: PERFORMANCE OPTIMIZATION:<br/>Skip blocking CLI call
        else Comment markers detected
            Gate-->>Hook: true
            Hook->>CLI: runCommentChecker(hookInput)
            CLI->>CLI: Parse and analyze comments
            CLI-->>Hook: CheckResult (hasComments, message)
            
            alt hasComments = true
                Hook->>Hook: Append warning message to output
            end
            
            Hook-->>Agent: Return (with optional warning)
        end
    end
Loading

@greptile-apps
Copy link

greptile-apps bot commented Jan 4, 2026

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/hooks/comment-checker/index.ts">

<violation number="1" location="src/hooks/comment-checker/index.ts:23">
P2: Adding `&quot;--&quot;` as a comment marker will cause many false positives, reducing the effectiveness of the gating optimization. The `--` pattern appears frequently in non-comment contexts (CLI arguments like `--help`, git commands, npm scripts). Consider a more specific pattern like requiring `--` at the start of a line followed by a space (standard SQL comment format), or remove this marker if false positive rate is unacceptable.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 2 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/hooks/claude-code-hooks/post-tool-use.ts">

<violation number="1" location="src/hooks/claude-code-hooks/post-tool-use.ts:159">
P3: Inconsistent logging pattern. The log function expects the descriptive message as the first argument (see line 97), but here a short tag is passed first and the error message second. Consider combining them into a single descriptive message.</violation>
</file>

<file name="src/hooks/comment-checker/index.ts">

<violation number="1" location="src/hooks/comment-checker/index.ts:23">
P2: Adding trailing space to `&quot;-- &quot;` causes false negatives for Lua, Haskell, and Ada comments which don&#39;t require a space after `--`. This gating mechanism will skip the CLI for legitimate comments like `--comment` in these languages. Consider keeping `&quot;--&quot;` to avoid missing valid comments, or add language-specific detection logic.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@Skyline-23 Skyline-23 force-pushed the feat/comment-checker-gating branch 2 times, most recently from 66fc079 to 73f8967 Compare January 5, 2026 20:04
@junhoyeo junhoyeo self-requested a review January 6, 2026 21:04
@code-yeongyu
Copy link
Owner

Not sure why this is required. this is already covered by comment checker. am i missing something?

@Skyline-23
Copy link
Author

Thanks for the question! This isn’t about detection coverage (comment-checker already does that). The new gating is a perf optimization: if the file content/old+new strings/edits contain no comment markers, we skip invoking the comment-checker CLI entirely. That avoids a blocking process spawn for pure code changes and reduces latency, while keeping comment-checker as the source of truth whenever markers are present. The gating is intentionally conservative (false positives are acceptable; false negatives would be worse).

@Skyline-23 Skyline-23 force-pushed the feat/comment-checker-gating branch from 73f8967 to d0e64f6 Compare January 7, 2026 10:14
@Skyline-23 Skyline-23 force-pushed the feat/comment-checker-gating branch from d0e64f6 to 9e77eed Compare January 8, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants