Skip to content

refactor: improve and cleanup (unused) code - #788

Merged
stuebingerb merged 1 commit into
mainfrom
refactor/code-cleanup
Nov 13, 2026
Merged

stuebingerb merged 1 commit into
mainfrom
refactor/code-cleanup

Conversation

@stuebingerb

Copy link
Copy Markdown
Owner

Mainly:

  • Avoid list creation if not required
  • Otherwise, use collection builders to reduce amount of intermediate lists
  • Remove unused classes, functions and properties
  • Replace map with forEach when return value was not used

@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Explain Change →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 50 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

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: b388cc02-d2cf-439c-a356-bdfa635ee0e2
Commits

Reviewing files that changed from the base of the PR and between 2a73124 and 08c15b1.

Files selected for processing: 17

  • kgraphql-ktor-stitched/src/main/kotlin/de/stuebingerb/kgraphql/stitched/schema/structure/StitchedSchemaCompilation.kt
  • kgraphql-ktor/api/kgraphql-ktor.api
  • kgraphql-ktor/src/main/kotlin/de/stuebingerb/kgraphql/KtorFeature.kt
  • kgraphql-ktor/src/main/kotlin/de/stuebingerb/kgraphql/KtorGraphQLConfiguration.kt
  • kgraphql-ktor/src/test/kotlin/de/stuebingerb/kgraphql/Kraph.kt
  • kgraphql-ktor/src/test/kotlin/de/stuebingerb/kgraphql/KtorMultipleEndpointsTest.kt
  • kgraphql/api/kgraphql.api
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/execution/Execution.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/execution/ParallelRequestExecutor.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/DataLoader.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/Deprecation.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/MutableSchemaDefinition.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/ast/Location.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaCompilation.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaModel.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/blockString.kt
  • kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/DataLoaderTest.kt

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • API Changes
    • Removed the Ktor configuration type and several public schema-model APIs, including data-loader utilities and query-type lookup. Applications relying on these APIs may need updates.
  • Refactor
    • Streamlined schema and query processing while preserving the resulting type collections and path components.
  • Bug Fixes
    • Updated subscription argument handling and playground resource loading. Also refined asynchronous test timing and execution.

Schema construction and runtime code now use updated collection and iteration patterns. Several model API declarations and the Ktor configuration class are removed. Ktor playground resource loading and test support are updated.

Changes: Core schema and runtime

Layer / File(s) Summary
Schema type construction and validation
kgraphql-ktor-stitched/.../StitchedSchemaCompilation.kt, kgraphql/src/main/.../MutableSchemaDefinition.kt, kgraphql/src/main/.../SchemaCompilation.kt
Schema compilation and stitched schema compilation build type or field collections with list builders. Type-name checks inspect the relevant collections directly. Built-in enum definitions use enumEntries.
Execution and utility updates
kgraphql/src/main/.../Execution.kt, kgraphql/src/main/.../ParallelRequestExecutor.kt, kgraphql/src/main/.../ast/Location.kt, kgraphql/src/main/.../blockString.kt, kgraphql/src/test/.../DataLoaderTest.kt
Execution path construction, subscription argument collection, and utility iteration are updated. Loader construction uses the enclosing coroutine scope's job. Tests use explicit millisecond durations, and simpleChild runs its blocking sleep on Dispatchers.IO.
Model API removals
kgraphql/src/main/.../schema/model/DataLoader.kt, kgraphql/src/main/.../schema/model/Deprecation.kt, kgraphql/src/main/.../schema/structure/SchemaModel.kt, kgraphql/api/kgraphql.api
The DataLoader implementation and related declarations, Deprecation, SchemaModel.queryTypesByName, and corresponding published API declarations are removed. Definition remains in the API.

Ktor configuration and test support

Layer / File(s) Summary
Ktor configuration and playground resource loading
kgraphql-ktor/api/kgraphql-ktor.api, kgraphql-ktor/src/main/.../KtorGraphQLConfiguration.kt, kgraphql-ktor/src/main/.../KtorFeature.kt, kgraphql-ktor/src/test/.../KtorMultipleEndpointsTest.kt
KtorGraphQLConfiguration and its published API declaration are removed. The feature and test load playground.html through GraphQL's class loader.
Ktor test query builder updates
kgraphql-ktor/src/test/.../Kraph.kt
The test query builder removes the String and Int variable overloads. Its field and variable iteration now uses forEach.

Change: Refactor

Merge Risk: Minimal · previous review 2a731

This is a cleanup refactor with no identified behavior change or merge-blocking issue. Public API removals are intended and no problems were found with them.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title uses the conventional commits prefix "refactor:" and is 43 characters, under the 50-character guideline. It says the PR refactors code and removes unused code, which matches the changeset: r…
Description check Passed The description lists the PR's main changes: avoiding list creation, using collection builders, removing unused classes, functions and properties, and replacing map with forEach. These match the file-…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title uses the conventional commits prefix "refactor:" and is 43 characters, under the 50-character guideline. It says the PR refactors code and removes unused code, which matches the changeset: removal of unused classes such as DataLoader, Deprecation, and KtorGraphQLConfiguration, plus collection-builder and forEach changes in several files.

Full details: Description check

Explanation

The description lists the PR's main changes: avoiding list creation, using collection builders, removing unused classes, functions and properties, and replacing map with forEach. These match the file-level changes in the summary, such as buildList in SchemaCompilation, removed DataLoader.kt and Deprecation.kt, and forEach in Location.kt and Kraph.kt.

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

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: ea89fa4c-5462-4262-ab05-154674fe9570
Commits

Reviewing files that changed from the base of the PR and between 59ad2b2 and 2a73124.

Files selected for processing: 17

  • kgraphql-ktor-stitched/src/main/kotlin/de/stuebingerb/kgraphql/stitched/schema/structure/StitchedSchemaCompilation.kt
  • kgraphql-ktor/api/kgraphql-ktor.api
  • kgraphql-ktor/src/main/kotlin/de/stuebingerb/kgraphql/KtorFeature.kt
  • kgraphql-ktor/src/main/kotlin/de/stuebingerb/kgraphql/KtorGraphQLConfiguration.kt
  • kgraphql-ktor/src/test/kotlin/de/stuebingerb/kgraphql/Kraph.kt
  • kgraphql-ktor/src/test/kotlin/de/stuebingerb/kgraphql/KtorMultipleEndpointsTest.kt
  • kgraphql/api/kgraphql.api
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/execution/Execution.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/execution/ParallelRequestExecutor.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/DataLoader.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/Deprecation.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/MutableSchemaDefinition.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/ast/Location.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaCompilation.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaModel.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/blockString.kt
  • kgraphql/src/test/kotlin/de/stuebingerb/kgraphql/DataLoaderTest.kt
Files with no reviewable changes: 6

  • kgraphql-ktor/api/kgraphql-ktor.api
  • kgraphql-ktor/src/main/kotlin/de/stuebingerb/kgraphql/KtorGraphQLConfiguration.kt
  • kgraphql/api/kgraphql.api
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/DataLoader.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/model/Deprecation.kt
  • kgraphql/src/main/kotlin/de/stuebingerb/kgraphql/schema/structure/SchemaModel.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.

@codecov

codecov Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.26%. Comparing base (59ad2b2) to head (08c15b1).

Files with missing lines Patch % Lines
...main/kotlin/de/stuebingerb/kgraphql/KtorFeature.kt 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #788      +/-   ##
==========================================
+ Coverage   84.49%   85.26%   +0.77%     
==========================================
  Files         151      148       -3     
  Lines        5113     5082      -31     
  Branches      891      886       -5     
==========================================
+ Hits         4320     4333      +13     
+ Misses        491      448      -43     
+ Partials      302      301       -1     

☔ 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.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 17 files

Architecture diagram
sequenceDiagram
    participant Client as GraphQL Client
    participant Ktor as Ktor Application
    participant GraphQL as GraphQL Plugin
    participant Schema as Schema Compilation
    participant Model as Schema Model
    participant Executor as ParallelRequestExecutor
    participant DataLoader as DataLoader (external lib)

    Note over Client,Executor: GraphQL Request Flow with Schema Compilation

    Client->>Ktor: HTTP POST /graphql (query)
    Ktor->>GraphQL: handleRequest()
    GraphQL->>Schema: compile/validate schema
    Schema->>Schema: buildList() for types
    Note over Schema: Efficient collection building<br/>without intermediate lists
    Schema->>Model: SchemaModel(allTypes)
    Model->>Model: Build type lookup maps
    GraphQL->>Executor: execute query

    Executor->>Executor: Parse execution tree
    Executor->>DataLoader: batch load keys
    Note over Executor,DataLoader: NEW: coroutineContext reference fix
    DataLoader->>Executor: batched results

    alt Has receiver
        Executor->>Executor: invoke directly
    else Subscription
        Executor->>Executor: getSubscriptionArgs() (exhaustive when)
    end

    Executor->>GraphQL: Execution results
    GraphQL-->>Ktor: GraphQL response
    Ktor-->>Client: HTTP 200 (JSON)
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Mainly:
- Avoid list creation if not required
- Otherwise, use collection builders to reduce amount of intermediate
  lists
- Remove unused classes, functions and properties
- Replace `map` with `forEach` when return value was not used
@stuebingerb
stuebingerb force-pushed the refactor/code-cleanup branch from 2a73124 to 08c15b1 Compare November 13, 2026 14:26
@github-actions

Copy link
Copy Markdown

🐰 Bencher Report

ProjectKGraphQL
Branchrefactor/code-cleanup
Testbedubuntu-latest
Click to view all benchmark results
BenchmarkThroughputBenchmark Result
operations / second (ops/s)
(Result Δ%)
Lower Boundary
operations / second (ops/s)
(Limit %)
de.stuebingerb.kgraphql.FunctionExecutionBenchmark.benchmarkFunctionExecution📈 view plot
🚷 view threshold
5,572,795.49 ops/s
(-8.33%)Baseline: 6,078,930.50 ops/s
3,770,456.77 ops/s
(67.66%)
de.stuebingerb.kgraphql.ParallelExecutionBenchmark.queryBenchmark📈 view plot
🚷 view threshold
1.30 ops/s
(+0.10%)Baseline: 1.30 ops/s
1.29 ops/s
(99.53%)
de.stuebingerb.kgraphql.QueryBenchmark.executionError📈 view plot
🚷 view threshold
18,170.51 ops/s
(-14.79%)Baseline: 21,325.34 ops/s
5,939.24 ops/s
(32.69%)
de.stuebingerb.kgraphql.QueryBenchmark.inputFromDocument📈 view plot
🚷 view threshold
25,662.00 ops/s
(-2.85%)Baseline: 26,414.72 ops/s
10,761.92 ops/s
(41.94%)
de.stuebingerb.kgraphql.QueryBenchmark.inputFromVariable📈 view plot
🚷 view threshold
24,852.48 ops/s
(-3.35%)Baseline: 25,713.62 ops/s
10,149.35 ops/s
(40.84%)
de.stuebingerb.kgraphql.QueryBenchmark.largeList📈 view plot
🚷 view threshold
4.43 ops/s
(-7.76%)Baseline: 4.81 ops/s
3.24 ops/s
(73.04%)
de.stuebingerb.kgraphql.QueryBenchmark.largeListWithFragment📈 view plot
🚷 view threshold
4.96 ops/s
(-7.82%)Baseline: 5.38 ops/s
3.70 ops/s
(74.63%)
de.stuebingerb.kgraphql.QueryBenchmark.manyChildren📈 view plot
🚷 view threshold
185.13 ops/s
(-3.64%)Baseline: 192.13 ops/s
91.27 ops/s
(49.30%)
de.stuebingerb.kgraphql.QueryBenchmark.manyChildrenWithFragment📈 view plot
🚷 view threshold
162.93 ops/s
(-20.91%)Baseline: 206.00 ops/s
88.88 ops/s
(54.55%)
de.stuebingerb.kgraphql.QueryBenchmark.manyDataChildren📈 view plot
🚷 view threshold
8.88 ops/s
(-0.72%)Baseline: 8.94 ops/s
8.61 ops/s
(97.06%)
de.stuebingerb.kgraphql.QueryBenchmark.manyOperations📈 view plot
🚷 view threshold
313.35 ops/s
(-3.26%)Baseline: 323.91 ops/s
141.94 ops/s
(45.30%)
de.stuebingerb.kgraphql.QueryBenchmark.manyOperationsWithFragment📈 view plot
🚷 view threshold
320.66 ops/s
(-2.83%)Baseline: 330.00 ops/s
145.60 ops/s
(45.41%)
de.stuebingerb.kgraphql.QueryBenchmark.nestedObject📈 view plot
🚷 view threshold
8,033.32 ops/s
(-10.74%)Baseline: 8,999.46 ops/s
3,702.50 ops/s
(46.09%)
de.stuebingerb.kgraphql.RequestCachingBenchmark.invalidRequest📈 view plot
🚷 view threshold
162,296.86 ops/s
(+1.05%)Baseline: 160,615.96 ops/s
81,029.23 ops/s
(49.93%)
de.stuebingerb.kgraphql.RequestCachingBenchmark.largeRequest📈 view plot
🚷 view threshold
8,743.52 ops/s
(-7.53%)Baseline: 9,455.64 ops/s
3,850.97 ops/s
(44.04%)
de.stuebingerb.kgraphql.RequestCachingBenchmark.smallRequest📈 view plot
🚷 view threshold
12,251.70 ops/s
(-11.51%)Baseline: 13,845.96 ops/s
5,019.91 ops/s
(40.97%)
de.stuebingerb.kgraphql.SimpleExecutionOverheadBenchmark.benchmark📈 view plot
🚷 view threshold
444,590.81 ops/s
(-17.78%)Baseline: 540,763.16 ops/s
220,875.39 ops/s
(49.68%)
🐰 View full continuous benchmarking report in Bencher

@stuebingerb
stuebingerb merged commit 223a627 into main Nov 13, 2026
14 checks passed
@stuebingerb
stuebingerb deleted the refactor/code-cleanup branch November 13, 2026 14:57
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.

1 participant