ronantakizawa/codereview-bench huggingface dataset:
Repository: Infisical/infisical
File: frontend/vite.config.ts (TypeScript)

Code before:
    base: "/",
    server: {
      allowedHosts,
      host: true,
      port: 3000
      // proxy: {
      //   "/api": {
      //     target: "http://localhost:8080",
      //     changeOrigin: true,
      //     secure: false,
      //     ws: true
      //   }
      // }
    },
    build: {
      rollupOptions: {
        output: {
          entryFileNames: `assets/[name]-${version}-[hash].js`,
          chunkFileNames: `assets/[name]-${version}-[hash].js`,
          assetFileNames: `assets/[name]-${version}-[hash].[ext]`
        }
      }
    },
    experimental: {
      // Only apply CDN HOST to files in /assets/* directory
      renderBuiltUrl(filename) {
        if (filename.startsWith("assets/") && cdnHost) {
          return `${cdnHost}/${filename}`;
        }
        return `/${filename}`;
      }
    },
    plugins: [
      tsconfigPaths(),
      nodePolyfills({
        globals: {
          Buffer: true
        }
      }),
      wasm(),
      topLevelAwait(),
      TanStackRouterVite({
        virtualRouteConfig: "./src/routes.ts"
      }),
      react(),
      virtualRouteFileChangeReloadPlugin
    ]
  };
});


Review comment (suggestion):
Instead of this - can we try doing as said in documentation of using a window url to fetch assets.
```
experimental: {
  renderBuiltUrl(filename, { hostType }) {
    if (hostType === 'js') {
      return { runtime: `window.__toCdnUrl(${JSON.stringify(filename)})` }
    } else {
      return { relative: true }
    }
  },
},

```

Then we can utilize this and use just an environment variable to change it dynamically

<img width="941" height="631" alt="Image" src="https://github.com/user-attachments/assets/57d75334-433e-454d-b0db-cfeb320468e6" />

Code after:
      //     target: "http://localhost:8080",
      //     changeOrigin: true,
      //     secure: false,
      //     ws: true
      //   }
      // }
    },
    build: {
      rollupOptions: {
        output: {
          entryFileNames: `assets/[name]-${version}-[hash].js`,
          chunkFileNames: `assets/[name]-${version}-[hash].js`,
          assetFileNames: `assets/[name]-${version}-[hash].[ext]`
        }
      }
    },
    experimental: {
      renderBuiltUrl(filename, { hostType }) {
        if (hostType === "js") {
          return { runtime: `window.__toCdnUrl(${JSON.stringify(filename)})` };
        }

        return { relative: true };
      }
    },
    plugins: [
      tsconfigPaths(),
      nodePolyfills({
        globals: {
          Buffer: true
        }
      }),
      wasm(),
      topLevelAwait(),
      TanStackRouterVite({
        virtualRouteConfig: "./src/routes.ts"
      }),
      react(),
      virtualRouteFileChangeReloadPlugin
    ]
  };
});
Repository: Infisical/infisical
File: backend/src/ee/services/group/group-fns.ts (TypeScript)

Code before:
        membersToRemoveFromGroupNonPending.push(member);
      } else {
        // remove incomplete member from pending group addition
        membersToRemoveFromGroupPending.push(member);
      }
    });

    if (membersToRemoveFromGroupNonPending.length) {
      // check which projects the group is part of
      const projectIds = Array.from(
        new Set(
          (
            await membershipGroupDAL.find(
              {
                scope: AccessScope.Project,
                actorGroupId: group.id,
                scopeOrgId: group.orgId
              },
              { tx }
            )
          ).map((gp) => gp.scopeProjectId as string)
        )
      );

      for await (const userId of membersToRemoveFromGroupNonPending.map((m) => m.id)) {
        const t = await userGroupMembershipDAL.filterProjectsByUserMembership(userId, group.id, projectIds, tx);
        const projectsToDeleteKeyFor = projectIds.filter((p) => !t.has(p));

        if (projectsToDeleteKeyFor.length) {
          await projectKeyDAL.delete(
            {
              receiverId: userId,
              $in: {
                projectId: projectsToDeleteKeyFor
              }
            },
            tx
          );
        }

        await userGroupMembershipDAL.delete(
          {
            groupId: group.id,
            userId
          },
          tx
        );
      }
    }

    if (membersToRemoveFromGroupPending.length) {

Review comment (suggestion):
Can we please rename these variables `m, t, and p`. Makes it super hard to read whats actually going on

Code after:
        membersToRemoveFromGroupNonPending.push(member);
      } else {
        // remove incomplete member from pending group addition
        membersToRemoveFromGroupPending.push(member);
      }
    });

    if (membersToRemoveFromGroupNonPending.length) {
      // check which projects the group is part of
      const projectIds = Array.from(
        new Set(
          (
            await membershipGroupDAL.find(
              {
                scope: AccessScope.Project,
                actorGroupId: group.id,
                scopeOrgId: group.orgId
              },
              { tx }
            )
          ).map((gp) => gp.scopeProjectId as string)
        )
      );

      for await (const userId of membersToRemoveFromGroupNonPending.map((member) => member.id)) {
        const projectsUserStillMemberOf = await userGroupMembershipDAL.filterProjectsByUserMembership(
          userId,
          group.id,
          projectIds,
          tx
        );
        const projectsToDeleteKeyFor = projectIds.filter((projectId) => !projectsUserStillMemberOf.has(projectId));

        if (projectsToDeleteKeyFor.length) {
          await projectKeyDAL.delete(
            {
              receiverId: userId,
              $in: {
                projectId: projectsToDeleteKeyFor
              }
            },
            tx
          );
        }

        await userGroupMembershipDAL.delete(
          {
            groupId: group.id,
            userId
          },
          tx
Repository: Infisical/infisical
File: backend/src/services/certificate-v3/certificate-v3-fns.ts (TypeScript)

Code before:
  }

  // Priority 2: Profile's defaultTtlDays
  if (profileDefaultTtlDays) {
    // Validate against policy's maxValidity (catch config drift)
    if (policyMaxValidity) {
      const profileTtlMs = profileDefaultTtlDays * 24 * 60 * 60 * 1000;
      const policyMaxMs = ms(policyMaxValidity);

      if (profileTtlMs > policyMaxMs) {
        throw new BadRequestError({
          message: `Profile's default TTL (${profileDefaultTtlDays} days) exceeds the policy's maximum validity (${policyMaxValidity}). Please update the profile or policy to fix this configuration mismatch.`
        });
      }
    }
    return `${profileDefaultTtlDays}d`;
  }

  // Priority 3: Flow default (for ACME, EST, etc.)
  if (flowDefaultTtl) {
    return flowDefaultTtl;
  }

  // No TTL source available - throw error for API flows
  throw new BadRequestError({
    message: "TTL is required. Either pass a TTL in the request or set a default TTL on the certificate profile."
  });
};


Review comment (suggestion):
mmm maybe we can rephrase this to be something like certificates can not be issued without a proper TTL? (just feels a bit weird to mention to pass a parameter or set the profile property when this can be used on very different flows 🤔 )

Code after:
  }

  // Priority 2: Profile's defaultTtlDays
  if (profileDefaultTtlDays) {
    // Validate against policy's maxValidity (catch config drift)
    if (policyMaxValidity) {
      const profileTtlMs = profileDefaultTtlDays * 24 * 60 * 60 * 1000;
      const policyMaxMs = ms(policyMaxValidity);

      if (profileTtlMs > policyMaxMs) {
        throw new BadRequestError({
          message: `Profile's default TTL (${profileDefaultTtlDays} days) exceeds the policy's maximum validity (${policyMaxValidity}). Please update the profile or policy to fix this configuration mismatch.`
        });
      }
    }
    return `${profileDefaultTtlDays}d`;
  }

  // Priority 3: Flow default (for ACME, EST, etc.)
  if (flowDefaultTtl) {
    return flowDefaultTtl;
  }

  // No TTL source available - throw error for API flows
  throw new BadRequestError({
    message: "Certificate issuance requires a valid TTL"
  });
};
Repository: InsertKoinIO/koin
File: koin-projects/examples/multimodule-ktor/module-b/src/main/kotlin/org/koin/samples/multi_module/module_b/moduleB.kt (Kotlin)

Code before:
package org.koin.samples.multi_module.module_b

import io.ktor.application.Application
import io.ktor.application.call
import io.ktor.response.respondText
import io.ktor.routing.get
import io.ktor.routing.routing
import org.koin.dsl.bind
import org.koin.dsl.module
import org.koin.experimental.builder.single
import org.koin.ktor.ext.inject
import org.koin.ktor.ext.koinModules
import org.koin.samples.multi_module.common.IService

fun Application.moduleB() {
    koinModules(
            module {
                single<ModuleBService>() bind IService::class
            }
    )
    routing {
        val service by inject<ModuleBService>()
        get("/module-b") {
            call.respondText(service.sayHello())
        }
    }

}

Review comment (suggestion):
perhaps just `modules` instead of `koinModules` in fact :)

Code after:
package org.koin.samples.multi_module.module_b

import io.ktor.application.Application
import io.ktor.application.call
import io.ktor.response.respondText
import io.ktor.routing.get
import io.ktor.routing.routing
import org.koin.dsl.bind
import org.koin.dsl.module
import org.koin.experimental.builder.single
import org.koin.ktor.ext.inject
import org.koin.ktor.ext.modules
import org.koin.samples.multi_module.common.IService

fun Application.moduleB() {
    modules(
            module {
                single<ModuleBService>() bind IService::class
            }
    )
    routing {
        val service by inject<ModuleBService>()
        get("/module-b") {
            call.respondText(service.sayHello())
        }
    }

}
Repository: InsertKoinIO/koin
File: koin-projects/examples/androidx-samples/src/test/java/CheckModulesTest.kt (Kotlin)

Code before:
import org.junit.Test
import org.junit.experimental.categories.Category
import org.koin.core.parameter.parametersOf
import org.koin.sample.androidx.components.mvp.FactoryPresenter
import org.koin.sample.androidx.components.mvp.ScopedPresenter
import org.koin.sample.androidx.di.appModule
import org.koin.sample.androidx.di.mvpModule
import org.koin.sample.androidx.di.mvvmModule
import org.koin.test.category.CheckModuleTest
import org.koin.test.check.checkModules

@Category(CheckModuleTest::class)
class CheckModulesTest {

    @Test
    fun `test DI modules`() = checkModules(
            parameters = {
                create<FactoryPresenter> { parametersOf("_ID_") }
                create<ScopedPresenter> { parametersOf("_ID_") }
            }
    ) {
        modules(appModule + mvpModule + mvvmModule)// + mvpModule + mvvmModule + scopeModule)
    }

}

Review comment (suggestion):
Remove `mvvmModule` to fix tests:
```suggestion
        modules(appModule + mvpModule)
```

Or provide parameters to dependencies from `mvvmModule`:
```kotlin
create<SimpleViewModel> { parametersOf("_ID_") }
create<SimpleViewModel>(named("vm1")) { parametersOf("_ID_") }
create<SimpleViewModel>(named("vm2")) { parametersOf("_ID_") }
create<SavedStateViewModel> { parametersOf(SavedStateHandle(), "_ID_") }
create<SavedStateViewModel>(named("vm2")) { parametersOf(SavedStateHandle(), "_ID_") }
```

(https://circleci.com/gh/InsertKoinIO/koin/625?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link)

Code after:
import org.junit.Test
import org.junit.experimental.categories.Category
import org.koin.core.parameter.parametersOf
import org.koin.sample.androidx.components.mvp.FactoryPresenter
import org.koin.sample.androidx.components.mvp.ScopedPresenter
import org.koin.sample.androidx.di.appModule
import org.koin.sample.androidx.di.mvpModule
import org.koin.sample.androidx.di.mvvmModule
import org.koin.test.category.CheckModuleTest
import org.koin.test.check.checkModules

@Category(CheckModuleTest::class)
class CheckModulesTest {

    @Test
    fun `test DI modules`() = checkModules(
            parameters = {
                create<FactoryPresenter> { parametersOf("_ID_") }
                create<ScopedPresenter> { parametersOf("_ID_") }
            }
    ) {
        modules(appModule + mvpModule)
    }

}
Repository: InsertKoinIO/koin
File: koin-projects/koin-core/src/main/kotlin/org/koin/core/time/Measure.kt (Kotlin)

Code before:
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.koin.core.time


/**
 * Measure functions
 *
 * @author Arnaud Giuliani
 */

/**
 * Measure code execution
 */
fun measureDuration(code: () -> Unit): Double {
    val initialTime = System.nanoTime().toDouble()
    code()
    return (System.nanoTime() - initialTime).inMilliseconds()
}

fun measureDuration(message: String, code: () -> Unit) {
    val time = measureDuration(code)
    println("$message - $time ms")
}

/**
 * Measure code execution and get result
 */
fun <T> measureDurationForResult(code: () -> T): Pair<T, Double> {
    val initialTime = System.nanoTime().toDouble()
    val result = code()
    val elapsedTime = (System.nanoTime() - initialTime).inMilliseconds()
    return Pair(result, elapsedTime)
}

fun <T> measureDurationForResult(message: String, code: () -> T): T {
    val (result, time) = measureDurationForResult(code)
    println("$message - $time ms")
    return result
}

private fun Double.inMilliseconds(): Double = (this / 1000000)


Review comment (refactor):
just simple extract as `startTime` & `endTime` those variables 👍 

Code after:
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.koin.core.time


/**
 * Measure functions
 *
 * @author Arnaud Giuliani
 */

/**
 * Measure code execution
 */
fun measureDuration(code: () -> Unit): Double {
    val startTime = System.nanoTime().toDouble()
    code()
    val endTime = System.nanoTime().toDouble()
    return (endTime - startTime).inMilliseconds()
}

fun measureDuration(message: String, code: () -> Unit) {
    val time = measureDuration(code)
    println("$message - $time ms")
}

/**
 * Measure code execution and get result
 */
fun <T> measureDurationForResult(code: () -> T): Pair<T, Double> {
    val startTime = System.nanoTime().toDouble()
    val result = code()
    val endTime = System.nanoTime().toDouble()
    return Pair(result, (endTime - startTime).inMilliseconds())
}

fun <T> measureDurationForResult(message: String, code: () -> T): T {
    val (result, time) = measureDurationForResult(code)
    println("$message - $time ms")
    return result
}

private fun Double.inMilliseconds(): Double = (this / 1000000)
Repository: InsertKoinIO/koin
File: koin-projects/koin-core/src/main/kotlin/org/koin/core/time/Measure.kt (Kotlin)

Code before:
 * Measure functions
 *
 * @author Arnaud Giuliani
 */

/**
 * Measure code execution
 */
fun measureDuration(code: () -> Unit): Double {
    val initialTime = System.nanoTime().toDouble()
    code()
    return (System.nanoTime() - initialTime).inMilliseconds()
}

fun measureDuration(message: String, code: () -> Unit) {
    val time = measureDuration(code)
    println("$message - $time ms")
}

/**
 * Measure code execution and get result
 */
fun <T> measureDurationForResult(code: () -> T): Pair<T, Double> {
    val initialTime = System.nanoTime().toDouble()
    val result = code()
    val elapsedTime = (System.nanoTime() - initialTime).inMilliseconds()
    return Pair(result, elapsedTime)
}

fun <T> measureDurationForResult(message: String, code: () -> T): T {
    val (result, time) = measureDurationForResult(code)
    println("$message - $time ms")
    return result
}

private fun Double.inMilliseconds(): Double = (this / 1000000)


Review comment (refactor):
just simple extract as `startTime` & `endTime` those variables 👍 

Code after:
 * Measure functions
 *
 * @author Arnaud Giuliani
 */

/**
 * Measure code execution
 */
fun measureDuration(code: () -> Unit): Double {
    val startTime = System.nanoTime().toDouble()
    code()
    val endTime = System.nanoTime().toDouble()
    return (endTime - startTime).inMilliseconds()
}

fun measureDuration(message: String, code: () -> Unit) {
    val time = measureDuration(code)
    println("$message - $time ms")
}

/**
 * Measure code execution and get result
 */
fun <T> measureDurationForResult(code: () -> T): Pair<T, Double> {
    val startTime = System.nanoTime().toDouble()
    val result = code()
    val endTime = System.nanoTime().toDouble()
    return Pair(result, (endTime - startTime).inMilliseconds())
}

fun <T> measureDurationForResult(message: String, code: () -> T): T {
    val (result, time) = measureDurationForResult(code)
    println("$message - $time ms")
    return result
}

private fun Double.inMilliseconds(): Double = (this / 1000000)
Repository: InsertKoinIO/koin
File: projects/core/koin-core/src/commonMain/kotlin/org/koin/core/module/Module.kt (Kotlin)

Code before:
inline fun <reified T> _scopedInstanceFactory(
    qualifier: Qualifier? = null,
    noinline definition: Definition<T>,
    scopeQualifier: Qualifier,
): ScopedInstanceFactory<T> {
    val def = _createDefinition(Kind.Scoped, qualifier, definition, scopeQualifier = scopeQualifier)
    return ScopedInstanceFactory(def)
}

/**
 * Help write list of Modules
 */
operator fun List<Module>.plus(module: Module): List<Module> = this + listOf(module)

/**
 * Run through the module list to flatten all modules & submodules
 */
@OptIn(KoinInternalApi::class)
fun flatten(modules: List<Module>): Set<Module> {    
    val flatten = HashSet<Module>()
    val stack = ArrayDeque<Module>(modules)

    while (stack.isNotEmpty()) {
        val current = stack.removeLast()
        flatten.add(current)
        current.includedModules.forEach {
            if (it !in flatten) {
                stack.addLast(it)
            }
        }
    }

    return flatten
}


Review comment (suggestion):
Please use a regular `for` loop instead, `forEach` is intended to be used in "long call chains" or if the receiver is `nullable`.

For a reference, please see the [Kotlin Coding Conventions](https://kotlinlang.org/docs/coding-conventions.html#loops):
> (...) `forEach` (prefer using a regular for loop instead, unless the receiver of `forEach` is nullable or `forEach` is used as part of a longer call chain. (...)

Code after:
inline fun <reified T> _scopedInstanceFactory(
    qualifier: Qualifier? = null,
    noinline definition: Definition<T>,
    scopeQualifier: Qualifier,
): ScopedInstanceFactory<T> {
    val def = _createDefinition(Kind.Scoped, qualifier, definition, scopeQualifier = scopeQualifier)
    return ScopedInstanceFactory(def)
}

/**
 * Help write list of Modules
 */
operator fun List<Module>.plus(module: Module): List<Module> = this + listOf(module)

/**
 * Run through the module list to flatten all modules & submodules
 */
@OptIn(KoinInternalApi::class)
fun flatten(modules: List<Module>): Set<Module> {
    // This is actually a DFS traversal of the module graph,
    // but we're using a stack instead of recursion to avoid stack overflows and performance overhead.

    val flatten = HashSet<Module>()
    val stack = ArrayDeque(modules)

    while (stack.isNotEmpty()) {
        val current = stack.removeLast()

        // If the module is already in the set, that means we've already visited it, so we can skip it.
        if (!flatten.add(current)) {
            continue
        }

        // Add all the included modules to the stack if they haven't been visited yet.
        for (module in current.includedModules) {
            if (module !in flatten) {
                stack += module
            }
        }
    }

    return flatten
}
Repository: InsertKoinIO/koin
File: examples/jvm-perfs/src/main/kotlin/org/koin/benchmark/nestedModules.kt (Kotlin)

Code before:
): List<Module> {
    val matrix = List(depth) { List(width) { module {} } }

    val level0Index = matrix.indices.random()
    val level0 = matrix[level0Index]

    matrix.forEachIndexed { r, modules ->
        modules.forEachIndexed { c, module ->
            // Let each module include the one above it.
            matrix
                .getOrNull(r + 1)
                ?.get(c)
                ?.let { module.includes(it) }

            if (r != level0Index) {
                module.includes(level0)
            }
        }
    }

    return matrix.first()
}

@OptIn(KoinInternalApi::class)
internal fun flattenIterative(modules: List<Module>): Set<Module> {
    // This is essentially a DFS traversal of the module graph,
    // but we're using a stack instead of recursion to avoid stack overflows and performance overhead.

    val flatten = HashSet<Module>()
    val stack = ArrayDeque(modules)

    while (stack.isNotEmpty()) {
        val current = stack.removeLast()

        // If the module is already in the set, that means we've already visited it, so we can skip it.
        if (!flatten.add(current)) {
            continue
        }

        // Add all the included modules to the stack if they haven't been visited yet.
        for (module in current.includedModules) {
            if (module !in flatten) {
                stack += module
            }
        }
    }

    return flatten
}

Review comment (suggestion):
```suggestion
    // This is actually a DFS traversal of the module graph,
```

Code after:
): List<Module> {
    val matrix = List(depth) { List(width) { module {} } }

    val level0Index = matrix.indices.random()
    val level0 = matrix[level0Index]

    matrix.forEachIndexed { r, modules ->
        modules.forEachIndexed { c, module ->
            // Let each module include the one above it.
            matrix
                .getOrNull(r + 1)
                ?.get(c)
                ?.let { module.includes(it) }

            if (r != level0Index) {
                module.includes(level0)
            }
        }
    }

    return matrix.first()
}

@OptIn(KoinInternalApi::class)
internal fun flattenIterative(modules: List<Module>): Set<Module> {
    // This is actually a DFS traversal of the module graph,
    // but we're using a stack instead of recursion to avoid stack overflows and performance overhead.

    val flatten = HashSet<Module>()
    val stack = ArrayDeque(modules)

    while (stack.isNotEmpty()) {
        val current = stack.removeLast()

        // If the module is already in the set, that means we've already visited it, so we can skip it.
        if (!flatten.add(current)) {
            continue
        }

        // Add all the included modules to the stack if they haven't been visited yet.
        for (module in current.includedModules) {
            if (module !in flatten) {
                stack += module
            }
        }
    }

    return flatten
}