[stuebingerb]

Resolves #790

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Requests that declare fragments without using them are now correctly rejected, even after a previous request used a fragment with the same name.

InterpreterContext now owns directive lookup and fragment-use state. Directive resolution uses the context across operation locations, and execution-plan validation checks unused fragments through it. A regression test checks fragment validation across separate requests.

Changes: Request Interpreter State

Layer / File(s)	Summary
Context state, directive lookup, and fragment validation kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/RequestInterpreter.kt, kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/specification/language/FragmentsSpecificationTest.kt	InterpreterContext owns directive lookup and used-fragment state. Directive invocations across fragment spreads, inline fragments, fields, and remote operations resolve through the context. createExecutionPlan checks unused fragments through the context. The regression test verifies that a fragment used by one request is reported unused when a separate request only declares it.

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: Minimal · up to 39098

The change makes request interpretation independent between requests, so a fragment used in one request no longer hides an unused fragment in a later one. No merge-blocking risk is evident.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title "fix: make RequestInterpreter stateless" follows the conventional commits prefix format and names the main change: removing per-request state from RequestInterpreter. It is 38 characters, un…
Description check		The description "Resolves #790" links the PR to issue #790, which reports that RequestInterpreter retains directivesByName and usedFragments across requests. This relates to the changeset, although it…
Linked Issues check		[#790] The issue requires that RequestInterpreter not reuse directivesByName and usedFragments across requests, so a fragment used in one request is not treated as used in a later request. At th…
Out of Scope Changes check		All changes in the whole-PR diff (merge base to head) serve #790. The RequestInterpreter.kt changes move directive and fragment-usage state into InterpreterContext, update all lookup() call site…

Full details: Title check

Explanation

The title "fix: make RequestInterpreter stateless" follows the conventional commits prefix format and names the main change: removing per-request state from RequestInterpreter. It is 38 characters, under the 50-character guideline. The changeset moves directive lookup and used-fragment tracking into InterpreterContext, which matches the title.

Full details: Description check

Explanation

The description "Resolves #790" links the PR to issue #790, which reports that RequestInterpreter retains directivesByName and usedFragments across requests. This relates to the changeset, although it gives no implementation detail.

Full details: Linked Issues check

Explanation

[#790] The issue requires that RequestInterpreter not reuse directivesByName and usedFragments across requests, so a fragment used in one request is not treated as used in a later request. At the reviewed head, directivesByName and usedFragments moved from RequestInterpreter fields into InterpreterContext. createExecutionPlan constructs a new InterpreterContext for each call, so tracking state is per request. unusedFragments() computes unused fragments from the context's own usedFragments, and createExecutionPlan throws ValidationException("Found unused fragments: ...") on that result. Directive lookup goes through InterpreterContext.findDirective, which still throws Directive '...' does not exist for unknown names. The added test unused fragments should not use data from previous requests in FragmentsSpecificationTest.kt reproduces the issue scenario: the second request fails with Found unused fragments: [film_title]. Test and code cover the reported requirement. No unmet coding requirement found. Limitation: test results were not executed in this review.

Full details: Out of Scope Changes check

Explanation

All changes in the whole-PR diff (merge base to head) serve #790. The RequestInterpreter.kt changes move directive and fragment-usage state into InterpreterContext, update all lookup() call sites to pass ctx, and replace the inline unused-fragment filter with ctx.unusedFragments(). The regression test in FragmentsSpecificationTest.kt covers the reported behavior. One change is a line re-wrap of the applicableTypeNames assignment in validateForFragment. It has no behavioral effect and is minor formatting, so it is not treated as an unrelated change.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 75000bcf-3c56-421e-b3b2-5fbd087672c6

Commits

Reviewing files that changed from the base of the PR and between df09f08 and 39098c2.

Files selected for processing: 2

• kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/RequestInterpreter.kt
• kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/specification/language/FragmentsSpecificationTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[cubic-dev-ai]

No issues found across 2 files

Architecture diagram

sequenceDiagram
    participant Client as GraphQL Client
    participant Schema as Schema
    participant RI as RequestInterpreter
    participant Ctx as InterpreterContext
    participant VM as Validation Messages

    Note over Client,VM: RequestInterpreter is instantiated per schema, requests create fresh context

    Client->>RI: executeRequest(query)
    RI->>RI: createExecutionPlan(operation)
    RI->>Ctx: new InterpreterContext(fragments, variables)
    
    Note over RI,Ctx: Context holds directives map and usedFragments set (per-request state)
    
    RI->>RI: handleOperation()
    Ctx->>Ctx: Track fragment usage
    
    alt Fragment spread encountered
        RI->>Ctx: get(FragmentSpreadNode)
        Ctx->>Ctx: Check fragment stack for cycles
        Ctx-->>RI: Return Execution.Fragment
        Ctx->>Ctx: Record fragment as used
    end
    
    RI->>RI: handleSelectionField()
    
    alt Field has directives
        RI->>Ctx: findDirective(directiveNode)
        Ctx-->>RI: Directive definition or error
        alt Directive not found
            Ctx-->>VM: Throw ValidationException
        end
    end
    
    RI->>RI: Complete operation handling

    RI->>Ctx: unusedFragments()
    Ctx-->>RI: Fragments not used in this request only
    
    alt Unused fragments found
        RI-->>VM: Throw ValidationException("Found unused fragments")
        VM-->>Client: Validation error
    else All fragments used
        RI-->>Client: Execution plan
    end

Loading

_{Re-trigger cubic}

[codecov]

Codecov Report

❌ Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.26%. Comparing base (df09f08) to head (39098c2).

Files with missing lines	Patch %	Lines
...rb/kgraphql/schema/structure/RequestInterpreter.kt	64.28%	0 Missing and 5 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #791   +/-   ##
=======================================
  Coverage   85.26%   85.26%           
=======================================
  Files         148      148           
  Lines        5082     5083    +1     
  Branches      886      886           
=======================================
+ Hits         4333     4334    +1     
  Misses        448      448           
  Partials      301      301           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.