ronantakizawa/codereview-bench huggingface dataset:
Repository: denoland/fresh
File: update/src/update.ts (TypeScript)

Code before:
import * as path from "@std/path";
import * as JSONC from "@std/jsonc";
import * as tsmorph from "ts-morph";
import * as colors from "@std/fmt/colors";
import { ProgressBar } from "@std/cli/unstable-progress-bar";

export const SyntaxKind = tsmorph.ts.SyntaxKind;

export const FRESH_VERSION = "2.0.0-alpha.37";
export const PREACT_VERSION = "10.26.9";
export const PREACT_SIGNALS_VERSION = "2.2.1";

// Function to filter out node_modules and vendor directories from logs
function shouldHideFromLogs(filePath: string): boolean {
  const normalizedPath = filePath.replace(/\\/g, "/");
  return normalizedPath.includes("/node_modules/") ||
    normalizedPath.includes("/vendor/");
}

export interface DenoJson {
  lock?: boolean;
  tasks?: Record<string, string>;
  name?: string;
  version?: string;
  imports?: Record<string, string>;
}

async function format(filePath: string) {
  const command = new Deno.Command(Deno.execPath(), {
    args: ["fmt", filePath],
  });
  await command.output();
}

async function writeFormatted(filePath: string, content: string) {
  await Deno.writeTextFile(filePath, content);
  await format(filePath);
}

async function updateDenoJson(
  dir: string,
  fn: (json: DenoJson) => void | Promise<void>,
): Promise<void> {
  let filePath = path.join(dir, "deno.json");

Review comment (suggestion):
This can be simplified to:

```suggestion
const HIDE_FILES = /[\\/]+(node_modules|vendor)[\\/]+/;
```
And then later do `!HIDE_FILES.test(someFilePath)`.

Code after:
import * as path from "@std/path";
import * as JSONC from "@std/jsonc";
import * as tsmorph from "ts-morph";
import * as colors from "@std/fmt/colors";
import { ProgressBar } from "@std/cli/unstable-progress-bar";

export const SyntaxKind = tsmorph.ts.SyntaxKind;

export const FRESH_VERSION = "2.0.0-alpha.37";
export const PREACT_VERSION = "10.26.9";
export const PREACT_SIGNALS_VERSION = "2.2.1";

// Function to filter out node_modules and vendor directories from logs
const HIDE_FILES = /[\\/]+(node_modules|vendor)[\\/]+/;
export interface DenoJson {
  lock?: boolean;
  tasks?: Record<string, string>;
  name?: string;
  version?: string;
  imports?: Record<string, string>;
}

async function format(filePath: string) {
  const command = new Deno.Command(Deno.execPath(), {
    args: ["fmt", filePath],
  });
  await command.output();
}

async function writeFormatted(filePath: string, content: string) {
  await Deno.writeTextFile(filePath, content);
  await format(filePath);
}

async function updateDenoJson(
  dir: string,
  fn: (json: DenoJson) => void | Promise<void>,
): Promise<void> {
  let filePath = path.join(dir, "deno.json");
  try {
    const config = JSON.parse(await Deno.readTextFile(filePath)) as DenoJson;
    await fn(config);
    await writeFormatted(filePath, JSON.stringify(config));
    return;
Repository: denoland/fresh
File: www/utils/prism.ts (TypeScript)

Code before:
Prism.languages.sh.deno = {
  pattern: /(^|[\s;|&]|[<>]\()(?:deno)(?=$|[)\s;|&])/,
  lookbehind: true,
  alias: "function",
};
Prism.languages.bash.deno = Prism.languages.sh.deno;

/**
 * Adds `txt-files` language for file-structure code blocks.
 *
 * - Comments: Makes `#` and everything after a comment
 * - Operator: Matches the file structure symbols like `├──`, `└──`, and `│ `
 * - Root: Matches `<root>` or `<project root>` to indicate the root of the file structure
 * - Remaining text is rendered as plain text
 *
 * @example
 * ```txt-files
 * <project root>
 * ├── routes/             # File system based routes
 * │   ├── _app.tsx        # Renders the outer <html> content structure
 * │   └── index.tsx       # Renders /
 * ├── dev.ts     # Run this during development
 * └── main.ts    # Run this for production
 * ```
 */
Prism.languages["txt-files"] = {
  comment: {
    pattern: /#.*|$/,
    greedy: true,
  },
  operator: /├──|└──|│\s+ /,
  root: /<root>|<project root>/,
};

export { Prism };


Review comment (suggestion):
I didn't like that the myriad of "Project structure" code blocks was using `sh` (for comments coloring) or `txt`. Since #3162, this makes the icons vary and it looks odd when using `sh` for the icon.

The configuration here, together with the styling changes, both improves readability of the code blocks, but also makes it more consistent.

- Lowers line-height to make the gap between "lines" less obvious
- Highlights the project root (renamed from project dir, to match `BuildOptions.root` terminology)
- Makes the lines (`├──`, `└──` or `│   `) colored the same as operators
- Makes `#` until the end of the line comments

## Before/After

<img width="1641" height="647" alt="image" src="https://github.com/user-attachments/assets/e64a8f99-3613-4268-862f-2d9071662f41" />

<img width="1659" height="769" alt="image" src="https://github.com/user-attachments/assets/56846031-e162-4c25-9baf-207caa4371f3" />



Code after:
Prism.languages.sh.deno = {
  pattern: /(^|[\s;|&]|[<>]\()(?:deno)(?=$|[)\s;|&])/,
  lookbehind: true,
  alias: "function",
};
Prism.languages.bash.deno = Prism.languages.sh.deno;

/**
 * Adds `txt-files` language for file-structure code blocks.
 *
 * - Comments: Makes `#` and everything after a comment
 * - Operator: Matches the file structure symbols like `├──`, `└──`, and `│ `
 * - Root: Matches `<root>` or `<project root>` to indicate the root of the file structure
 * - Remaining text is rendered as plain text
 *
 * @example
 * ```txt-files
 * <project root>
 * ├── routes/             # File system based routes
 * │   ├── _app.tsx        # Renders the outer <html> content structure
 * │   └── index.tsx       # Renders /
 * ├── dev.ts     # Run this during development
 * └── main.ts    # Run this for production
 * ```
 */
Prism.languages["txt-files"] = {
  comment: {
    pattern: /#.*|$/,
    greedy: true,
  },
  operator: /├──|└──|│\s+ /,
  root: {
    pattern: /<root>|<project root>/,
    alias: "function",
  },
};

export { Prism };
Repository: denoland/fresh
File: www/main_test.ts (TypeScript)

Code before:
import {
  withBrowser,
  withChildProcessServer,
} from "../packages/fresh/tests/test_utils.tsx";
import { expect } from "@std/expect";
import { retry } from "@std/async/retry";
import { buildVite } from "../packages/plugin-vite/tests/test_utils.ts";

async function runTests() {
  // Build the project
  await using result = await buildVite(import.meta.dirname!);

  Deno.test("CORS should not set on GET /fresh-badge.svg", async () => {
    await withChildProcessServer(
      {
        cwd: result.tmp,
        args: ["serve", "-A", "--port", "0", "_fresh/server.js"],
      },
      async (address) => {
        const resp = await fetch(`${address}/fresh-badge.svg`);
        await resp?.body?.cancel();
        expect(resp.headers.get("cross-origin-resource-policy")).toEqual(null);
      },
    );
  });

  Deno.test({
    name: "shows version selector",
    fn: async () => {
      await retry(async () => {
        await withChildProcessServer(
          {
            cwd: result.tmp,
            args: ["serve", "-A", "--port", "0", "_fresh/server.js"],

Review comment (suggestion):
Try [`Deno.test.beforeAll`](https://docs.deno.com/runtime/fundamentals/testing/#available-hooks) instead.

Code after:
import {
  withBrowser,
  withChildProcessServer,
} from "../packages/fresh/tests/test_utils.tsx";
import { expect } from "@std/expect";
import { retry } from "@std/async/retry";
import {
  buildVite,
  launchProd,
} from "../packages/plugin-vite/tests/test_utils.ts";

let result: Awaited<ReturnType<typeof buildVite>>;

Deno.test.beforeAll(async () => {
  result = await buildVite(import.meta.dirname!);
});

Deno.test.afterAll(async () => {
  await result?.[Symbol.asyncDispose]();
});

Deno.test({
  name: "CORS should not set on GET /fresh-badge.svg",
  sanitizeOps: false,
  sanitizeResources: false,
  fn: async () => {
    await launchProd({ cwd: result.tmp }, async (address) => {
      const resp = await fetch(`${address}/fresh-badge.svg`);
      await resp?.body?.cancel();
      expect(resp.headers.get("cross-origin-resource-policy")).toEqual(null);
    });
  },
});
Repository: denoland/fresh
File: packages/build-id/mod.ts (TypeScript)

Code before:
import { encodeHex } from "@std/encoding/hex";

export const DENO_DEPLOYMENT_ID: string | undefined = Deno.env.get(
  "DENO_DEPLOYMENT_ID",
);
const deploymentId = DENO_DEPLOYMENT_ID ||
  // For CI
  Deno.env.get("GITHUB_SHA") ||
  Deno.env.get("CI_COMMIT_SHA") ||
  crypto.randomUUID();
const buildIdHash = await crypto.subtle.digest(
  "SHA-1",
  new TextEncoder().encode(deploymentId),
);

export let BUILD_ID: string = encodeHex(buildIdHash);

export function setBuildId(buildId: string): void {
  BUILD_ID = buildId;
}


Review comment (suggestion):
I don't know if JSR respects `@private`, or if it even makes sense to make a "public" API marked as `@private` because consumers of Fresh should not use it 🤔 

Code after:
/**
 * Don't use this package directly. It's considered internal for Fresh.
 *
 * @example
 * ```ts
 * // `BUILD_ID` is based on the `DENO_DEPLOYMENT_ID` environment variable.
 * ```
 *
 * @see https://fresh.deno.dev/docs/deployment/docker
 *
 * @module
 * @private
 */

import { encodeHex } from "@std/encoding/hex";

export const DENO_DEPLOYMENT_ID: string | undefined = Deno.env.get(
  "DENO_DEPLOYMENT_ID",
);
const deploymentId = DENO_DEPLOYMENT_ID ||
  // For CI
  Deno.env.get("GITHUB_SHA") ||
  Deno.env.get("CI_COMMIT_SHA") ||
  crypto.randomUUID();
const buildIdHash = await crypto.subtle.digest(
  "SHA-1",
  new TextEncoder().encode(deploymentId),
);

export let BUILD_ID: string = encodeHex(buildIdHash);

export function setBuildId(buildId: string): void {
  BUILD_ID = buildId;
}
Repository: dnhkng/GlaDOS
File: glados/whisper_cpp_wrapper.py (Python)

Code before:
    "darwin": DarwinLibraryLoader,
    "cygwin": WindowsLibraryLoader,
    "win32": WindowsLibraryLoader,
    "msys": WindowsLibraryLoader,
}

load_library = loaderclass.get(sys.platform, PosixLibraryLoader)()


def add_library_search_dirs(other_dirs):
    """
    Add libraries to search paths.
    If library paths are relative, convert them to absolute with respect to this
    file's directory
    """
    for path in other_dirs:
        if not os.path.isabs(path):
            path = os.path.abspath(path)
        load_library.other_dirs.append(path)


del loaderclass

# End loader

add_library_search_dirs([])

# Begin libraries
_libs["whisper"] = load_library("whisper")

# 1 libraries
# End libraries

# No modules

__uint32_t = c_uint  # /usr/include/x86_64-linux-gnu/bits/types.h: 42

uint32_t = __uint32_t  # /usr/include/x86_64-linux-gnu/bits/stdint-uintn.h: 26

enum_ggml_log_level = c_int  # /home/dnhkng/Documents/Projects/whisper.cpp/ggml.h: 461

ggml_log_callback = CFUNCTYPE(
    UNCHECKED(None), enum_ggml_log_level, String, POINTER(None)
)  # /home/dnhkng/Documents/Projects/whisper.cpp/ggml.h: 1826


# /home/dnhkng/Documents/Projects/whisper.cpp/whisper.h: 77
class struct_whisper_context(Structure):
    pass



Review comment (suggestion):
I'd recommend checking in an empty `lib` folder in the root of the repo (just add a `.gitkeep` empty file inside), and copying the required files from the submodules there.

Does `./submodules/*` actually work from here? I'd expect `./` here to be `/path/to/project/root/glados/` whereas the access to the `submodules` folder presumes `./` is simply `/path/to/project/roo/`

Code after:
    "darwin": DarwinLibraryLoader,
    "cygwin": WindowsLibraryLoader,
    "win32": WindowsLibraryLoader,
    "msys": WindowsLibraryLoader,
}

load_library = loaderclass.get(sys.platform, PosixLibraryLoader)()


def add_library_search_dirs(other_dirs):
    """
    Add libraries to search paths.
    If library paths are relative, convert them to absolute with respect to this
    file's directory
    """
    for path in other_dirs:
        if not os.path.isabs(path):
            path = os.path.abspath(path)
        load_library.other_dirs.append(path)


del loaderclass

# End loader

add_library_search_dirs(['./submodules/whisper.cpp/'])  # add the path to the whisper.cpp submodule

# Begin libraries
_libs["whisper"] = load_library("whisper")

# 1 libraries
# End libraries

# No modules

__uint32_t = c_uint  # /usr/include/x86_64-linux-gnu/bits/types.h: 42

uint32_t = __uint32_t  # /usr/include/x86_64-linux-gnu/bits/stdint-uintn.h: 26

enum_ggml_log_level = c_int  # /home/dnhkng/Documents/Projects/whisper.cpp/ggml.h: 461

ggml_log_callback = CFUNCTYPE(
    UNCHECKED(None), enum_ggml_log_level, String, POINTER(None)
)  # /home/dnhkng/Documents/Projects/whisper.cpp/ggml.h: 1826


# /home/dnhkng/Documents/Projects/whisper.cpp/whisper.h: 77
class struct_whisper_context(Structure):
    pass
Repository: dogecoin/dogecoin
File: src/wallet/init.cpp (C++)

Code before:

        return true;
    }

    if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) {
        LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__);
    }

    if (gArgs.IsArgSet("-zapwallettxes")) {
        return InitError(Untranslated("-zapwallettxes has been removed. If you are attempting to remove a stuck transaction from your wallet, please use abandontransaction instead."));
    }

    if (gArgs.GetBoolArg("-sysperms", false))
        return InitError(Untranslated("-sysperms is not allowed in combination with enabled wallet functionality"));

    return true;
}

void WalletInit::Construct(NodeContext& node) const
{
    ArgsManager& args = *Assert(node.args);
    if (args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) {
        LogPrintf("Wallet disabled!\n");
        return;
    }
    auto wallet_client = interfaces::MakeWalletClient(node, *node.chain, args);
    node.wallet_client = wallet_client.get();
    node.chain_clients.emplace_back(std::move(wallet_client));
}


Review comment (bug):
Will there ever be a situation where you would want to pass a  `chain` that differs from the chain referenced by `node`? If not, then this seems to add a potential place for someone to do something incorrectly, compared to something like `interfaces:MakeWalletClient(node, args);`. 

This would also lightly go for the same for `nodeContext` and `chain` in `WalletContext` - `chain` would ultimately just be a shortcut for nodeContext->chain, but that one would be a much more impactful change

Code after:

        return true;
    }

    if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) {
        LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__);
    }

    if (gArgs.IsArgSet("-zapwallettxes")) {
        return InitError(Untranslated("-zapwallettxes has been removed. If you are attempting to remove a stuck transaction from your wallet, please use abandontransaction instead."));
    }

    if (gArgs.GetBoolArg("-sysperms", false))
        return InitError(Untranslated("-sysperms is not allowed in combination with enabled wallet functionality"));

    return true;
}

void WalletInit::Construct(NodeContext& node) const
{
    ArgsManager& args = *Assert(node.args);
    if (args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) {
        LogPrintf("Wallet disabled!\n");
        return;
    }
    auto wallet_client = interfaces::MakeWalletClient(node, args);
    node.wallet_client = wallet_client.get();
    node.chain_clients.emplace_back(std::move(wallet_client));
}
Repository: dogecoin/dogecoin
File: contrib/gitian-build.sh (Shell)

Code before:
# Copyright (c) 2021 The dogecoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

# What to do
sign=false
verify=false
build=false
setupenv=false

# Systems to build
linux=true
windows=true
osx=true

# Other Basic variables
SIGNER=
VERSION=
commit=false
url=https://github.com/dogecoin/dogecoin
proc=2
mem=2000
lxc=true
osslTarUrl=https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/osslsigncode/1.7.1-1/osslsigncode_1.7.1.orig.tar.gz
osslPatchUrl=https://bitcoincore.org/cfields/osslsigncode-Backports-to-1.7.1.patch
scriptName=$(basename -- "$0")

Review comment (suggestion):
```suggestion
# Copyright (c) 2016 The Bitcoin Core developers
# Copyright (c) 2021 The Dogecoin Core developers
```
Copyrights are additive, rather than replacing.

Code after:
# Copyright (c) 2016 The Bitcoin Core developers
# Copyright (c) 2021 The Dogecoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

# What to do
sign=false
verify=false
build=false
setupenv=false

# Systems to build
linux=true
windows=true
osx=true

# Other Basic variables
SIGNER=
VERSION=
commit=false
url=https://github.com/dogecoin/dogecoin
proc=2
mem=2000
lxc=true
osslTarUrl=https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/osslsigncode/1.7.1-1/osslsigncode_1.7.1.orig.tar.gz
osslPatchUrl=https://bitcoincore.org/cfields/osslsigncode-Backports-to-1.7.1.patch
Repository: dogecoin/dogecoin
File: src/chain.cpp (C++)

Code before:
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <chain.h>

// Dogecoin: Include the single function signature we need, to avoid the entire
// validation.h being pulled in and creating a cyclic include loop
bool ReadBlockHeaderFromDisk(CBlockHeader& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams, const bool fCheckPOW = true);

/* Moved here from the header, because we need auxpow and the logic
   becomes more involved.  */
CBlockHeader CBlockIndex::GetBlockHeader(const Consensus::Params& consensusParams, bool fCheckPOW) const
{
    CBlockHeader block;

    block.nVersion       = nVersion;

    if (pprev)
        block.hashPrevBlock = pprev->GetBlockHash();
    block.hashMerkleRoot = hashMerkleRoot;
    block.nTime          = nTime;
    block.nBits          = nBits;
    block.nNonce         = nNonce;
    return block;
}

/**
 * CChain implementation
 */
void CChain::SetTip(CBlockIndex *pindex) {
    if (pindex == nullptr) {
        vChain.clear();
        return;

Review comment (suggestion):
I assume we need this in a later PR? I don't see this being used here. If yes, could you please bring it in with the later PR? Because we cannot solve a call hierarchy problem without having the code that introduces the problem.

Code after:
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <chain.h>

/* Moved here from the header, because we need auxpow and the logic
   becomes more involved.  */
CBlockHeader CBlockIndex::GetBlockHeader(const Consensus::Params& consensusParams, bool fCheckPOW) const
{
    CBlockHeader block;

    block.nVersion       = nVersion;

    if (pprev)
        block.hashPrevBlock = pprev->GetBlockHash();
    block.hashMerkleRoot = hashMerkleRoot;
    block.nTime          = nTime;
    block.nBits          = nBits;
    block.nNonce         = nNonce;
    return block;
}

/**
 * CChain implementation
 */
void CChain::SetTip(CBlockIndex *pindex) {
    if (pindex == nullptr) {
        vChain.clear();
        return;
    }
    vChain.resize(pindex->nHeight + 1);
    while (pindex && vChain[pindex->nHeight] != pindex) {
        vChain[pindex->nHeight] = pindex;
Repository: dogecoin/dogecoin
File: src/chain.cpp (C++)

Code before:
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <chain.h>

// Dogecoin: Include the single function signature we need, to avoid the entire
// validation.h being pulled in and creating a cyclic include loop
bool ReadBlockHeaderFromDisk(CBlockHeader& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams, const bool fCheckPOW = true);

/* Moved here from the header, because we need auxpow and the logic
   becomes more involved.  */
CBlockHeader CBlockIndex::GetBlockHeader(const Consensus::Params& consensusParams, bool fCheckPOW) const
{
    CBlockHeader block;

    block.nVersion       = nVersion;

    if (pprev)
        block.hashPrevBlock = pprev->GetBlockHash();
    block.hashMerkleRoot = hashMerkleRoot;
    block.nTime          = nTime;
    block.nBits          = nBits;
    block.nNonce         = nNonce;
    return block;
}

/**
 * CChain implementation
 */
void CChain::SetTip(CBlockIndex *pindex) {
    if (pindex == nullptr) {
        vChain.clear();
        return;
    }

Review comment (refactor):
Same comment as on #2547 .. the code that requires the include is not in so I cannot judge whether it's a good choice to do it like this, simply move some code to validation.cpp, or extract code that needs both chain.h and validation.h into another file. Can we postpone this until the code that requires this is actually added to chain.cpp?



Code after:
// Copyright (c) 2009-2010 Satoshi Nakamoto
// Copyright (c) 2009-2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <chain.h>

/* Moved here from the header, because we need auxpow and the logic
   becomes more involved.  */
CBlockHeader CBlockIndex::GetBlockHeader(const Consensus::Params& consensusParams, bool fCheckPOW) const
{
    CBlockHeader block;

    block.nVersion       = nVersion;

    if (pprev)
        block.hashPrevBlock = pprev->GetBlockHash();
    block.hashMerkleRoot = hashMerkleRoot;
    block.nTime          = nTime;
    block.nBits          = nBits;
    block.nNonce         = nNonce;
    return block;
}

/**
 * CChain implementation
 */
void CChain::SetTip(CBlockIndex *pindex) {
    if (pindex == nullptr) {
        vChain.clear();
        return;
    }
    vChain.resize(pindex->nHeight + 1);
    while (pindex && vChain[pindex->nHeight] != pindex) {
        vChain[pindex->nHeight] = pindex;
        pindex = pindex->pprev;