fix: omit introspection queries from complexity limits

[ysmolski]

This is disabled by default and should stay like that for now. There is a constant that can enable it again, but it is not intended to be tweaked by the clients.

Summary by CodeRabbit

• Bug Fixes

  • Operation complexity now correctly skips GraphQL introspection fields when configured, improving calculation accuracy.

• Tests

  • Added tests covering node-skipping behavior and cases where skipped fields contribute zero to node count, complexity, and depth.

[coderabbitai]

Walkthrough

Adds runtime skipping of GraphQL introspection fields (__schema and __type) in operation complexity middleware and renames a local directive-skip flag; updates tests and test schema to cover skipped nodes and an @nodeCountSkip field.

Changes

Cohort / File(s)	Summary
Middleware update v2/pkg/middleware/operation_complexity/operation_complexity.go	Adds skipIntrospection, __schemaLiteral, __typeLiteral; changes directive-skip variable naming (exits → skip); adds runtime check to skip __schema and __type fields during EnterField processing.
Tests & test schema v2/pkg/middleware/operation_complexity/operation_complexity_test.go	Adds __typename to a selection, introduces "skipped node" test expecting zero metrics for @nodeCountSkip field, removes @nodeCountSkip from __schema in the test schema and adds activeUsers: [User] @nodeCountSkip.
CI config .coderabbit.yaml	Adds a pre_merge_checks section with mode: off under reviews (docstrings only).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant Parser
  participant ComplexityMiddleware
  participant MetricsCollector

  Client->>Parser: send GraphQL request
  Parser->>ComplexityMiddleware: EnterField(field)
  alt field is introspection (__schema/__type) and skipIntrospection == true
    ComplexityMiddleware--x MetricsCollector: skip node counting
    note right of ComplexityMiddleware #B7E4C7: field ignored at runtime
  else check @nodeCountSkip directive
    alt directive present
      ComplexityMiddleware--x MetricsCollector: skip node counting
      note right of ComplexityMiddleware #FCE7C6: directive-based skip
    else
      ComplexityMiddleware->>MetricsCollector: record NodeCount/Complexity/Depth
    end
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "fix: omit introspection queries from complexity limits" directly and accurately describes the main change in the changeset. The code modifications add logic to skip introspection fields (__schema, __type) from complexity calculations by introducing a skipIntrospection constant and runtime checks, which aligns perfectly with the title's intent to omit introspection queries from complexity enforcement. The title is concise, specific, and uses clear language without vague terms, making it easily understandable to teammates reviewing the commit history.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch yury/eng-8046-omit-introspection-queries-from-complexity-limits

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c1b4462 and 18a977b.

📒 Files selected for processing (2)

• .coderabbit.yaml (1 hunks)
• v2/pkg/middleware/operation_complexity/operation_complexity.go (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• v2/pkg/middleware/operation_complexity/operation_complexity.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build and test (go 1.25 / windows-latest)
• GitHub Check: Build and test (go 1.25 / ubuntu-latest)
• GitHub Check: Build and test (go 1.25 / windows-latest)
• GitHub Check: Build and test (go 1.25 / ubuntu-latest)

🔇 Additional comments (1)

.coderabbit.yaml (1)

18-20: Clarify the rationale for disabling docstring pre-merge checks.

The addition of pre_merge_checks.docstrings.mode: off disables docstring coverage enforcement repository-wide. However, this configuration change appears unrelated to the PR's stated objective of omitting GraphQL introspection queries from complexity limits.

Verify:

• Is this configuration change intentional and directly related to the complexity limits fix?
• Should this be a permanent repository-wide setting, or should it be reverted after this PR is merged?
• If this is needed to unblock the current review, consider whether it should remain disabled long-term.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

v2/pkg/middleware/operation_complexity/operation_complexity.go (2)

2-24: Update documentation to reflect runtime introspection skipping.

The documentation at lines 22-23 mentions using @nodeCountSkip for introspection allowlisting, but the implementation now skips __schema and __type fields via a runtime check (lines 202-205). Consider updating the docs to clarify that introspection fields are skipped automatically, while @nodeCountSkip can be used for other fields.

Apply this diff to update the documentation:

 "nodeCountMultiply" indicates that the Int value the directive is applied on
 should be used as a Node multiplier.
 
-"nodeCountSkip" indicates that the algorithm should skip this Node.
-It can be used to allowlist certain query paths, e.g. for introspection.
+"nodeCountSkip" indicates that the algorithm should skip this Node.
+It can be used to allowlist certain query paths.
+
+Note: Introspection fields (__schema and __type) are automatically skipped
+from complexity calculations by default.

---

51-55: Consider making skipIntrospection configurable or document its purpose.

The skipIntrospection constant is set to true and the PR description mentions it's not intended to be modified by clients. If this is for internal debugging/testing only, consider adding a comment explaining this. Alternatively, if there's a use case for disabling introspection skipping, consider making it a configuration option rather than a package-level constant.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88ac8b8 and c1b4462.

⛔ Files ignored due to path filters (1)

• go.work.sum is excluded by !**/*.sum

📒 Files selected for processing (2)

• v2/pkg/middleware/operation_complexity/operation_complexity.go (3 hunks)
• v2/pkg/middleware/operation_complexity/operation_complexity_test.go (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (1)

v2/pkg/middleware/operation_complexity/operation_complexity.go (2)

• OperationStats (33-37)
• RootFieldStats (39-44)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build and test (go 1.25 / ubuntu-latest)
• GitHub Check: Build and test (go 1.25 / windows-latest)
• GitHub Check: Build and test (go 1.25 / ubuntu-latest)
• GitHub Check: Build and test (go 1.25 / windows-latest)

🔇 Additional comments (4)

v2/pkg/middleware/operation_complexity/operation_complexity.go (1)

196-205: LGTM! Clean implementation of introspection field skipping.

The runtime check correctly identifies and skips __schema and __type fields when skipIntrospection is enabled. The implementation properly:

• Checks the directive-based skip first (preserving existing behavior)
• Uses the defined constants for field name comparison
• Skips only root-level introspection fields (__schema, __type) while allowing __typename in regular queries

v2/pkg/middleware/operation_complexity/operation_complexity_test.go (3)

42-42: Good addition for test coverage.

Adding __typename to the test query verifies that it's not incorrectly skipped by the introspection check. This is correct behavior since __typename is commonly used in regular queries for type discrimination, not just introspection.

---

70-86: Excellent test coverage for directive-based skipping.

This test case verifies that the @nodeCountSkip directive continues to work correctly alongside the new runtime introspection skipping. The expectations are correct: when a root field is skipped via the directive, all complexity metrics should be zero and RootFieldStats should be empty.

---

613-618: Schema changes align well with the implementation.

The removal of @nodeCountSkip from __schema (line 613) and addition of activeUsers with the directive (line 618) properly test both skipping mechanisms:

• Introspection fields (__schema, __type) are now skipped via runtime check
• The activeUsers field tests that directive-based skipping still works

This ensures both code paths are covered by tests.