ronantakizawa/codereview-bench huggingface dataset:
Repository: xenova/transformers.js
File: src/utils/maths.js (JavaScript)

Code before:
    // Compute the exponentials of the array values
    const exps = arr.map(x => Math.exp(x - maxVal));

    // Compute the sum of the exponentials
    // @ts-ignore
    const sumExps = exps.reduce((acc, val) => acc + val, 0);

    // Compute the softmax values
    const softmaxArr = exps.map(x => x / sumExps);

    return /** @type {T} */(softmaxArr);
}

/**
 * Calculates the logarithm of the softmax function for the input array.
 * @template {TypedArray|number[]} T
 * @param {T} arr The input array to calculate the log_softmax function for.
 * @returns {T} The resulting log_softmax array.
 */
export function log_softmax(arr) {
    // Compute the maximum value in the array
    const maxVal = max(arr)[0];

    // Compute the sum of the exponentials
    // @ts-ignore
    const sumExps = arr.reduce((acc, val) => acc + Math.exp(val - maxVal), 0);

    // Compute the log of the sum
    const logSum = Math.log(sumExps);

    // Compute the softmax values
    const logSoftmaxArr = arr.map(x => x - maxVal - logSum);

    return /** @type {T} */(logSoftmaxArr);
}

/**
 * Calculates the dot product of two arrays.
 * @param {number[]} arr1 The first array.
 * @param {number[]} arr2 The second array.
 * @returns {number} The dot product of arr1 and arr2.
 */
export function dot(arr1, arr2) {
    let result = 0;
    for (let i = 0; i < arr1.length; ++i) {
        result += arr1[i] * arr2[i];
    }
    return result;
}



Review comment (suggestion):
```suggestion
    // Compute the sum of the exponentials
    let sumExps = 0;
    for(let i = 0; i < arr.length; ++i) {
        sumExps += Math.exp(arr[i] - maxVal);
    }
```

Code after:
    // Compute the exponentials of the array values
    const exps = arr.map(x => Math.exp(x - maxVal));

    // Compute the sum of the exponentials
    // @ts-ignore
    const sumExps = exps.reduce((acc, val) => acc + val, 0);

    // Compute the softmax values
    const softmaxArr = exps.map(x => x / sumExps);

    return /** @type {T} */(softmaxArr);
}

/**
 * Calculates the logarithm of the softmax function for the input array.
 * @template {TypedArray|number[]} T
 * @param {T} arr The input array to calculate the log_softmax function for.
 * @returns {T} The resulting log_softmax array.
 */
export function log_softmax(arr) {
    // Compute the maximum value in the array
    const maxVal = max(arr)[0];

    // Compute the sum of the exponentials
    let sumExps = 0;
    for(let i = 0; i < arr.length; ++i) {
        sumExps += Math.exp(arr[i] - maxVal);
    }

    // Compute the log of the sum
    const logSum = Math.log(sumExps);

    // Compute the softmax values
    const logSoftmaxArr = arr.map(x => x - maxVal - logSum);

    return /** @type {T} */(logSoftmaxArr);
}

/**
 * Calculates the dot product of two arrays.
 * @param {number[]} arr1 The first array.
 * @param {number[]} arr2 The second array.
 * @returns {number} The dot product of arr1 and arr2.
 */
export function dot(arr1, arr2) {
    let result = 0;
    for (let i = 0; i < arr1.length; ++i) {
        result += arr1[i] * arr2[i];
    }
    return result;
}
Repository: xenova/transformers.js
File: examples/extension/src/background.js (JavaScript)

Code before:
// background.js - Handles requests from the UI, runs the model, then sends back a response

import { pipeline, env } from '@xenova/transformers';

// Skip initial check for local models, since we are not loading any local models.
env.allowLocalModels = false;

// Due to a bug in onnxruntime-web, we must disable multithreading for now.
// See https://github.com/microsoft/onnxruntime/issues/14445 for more information.
//env.backends.onnx.wasm.numThreads = 1;


class PipelineSingleton {
    static task = 'text-classification';
    static model = 'Xenova/distilbert-base-uncased-finetuned-sst-2-english';
    static instance = null;

    static async getInstance(progress_callback = null) {
        if (this.instance === null) {
            this.instance = pipeline(this.task, this.model, { progress_callback });
        }

        return this.instance;
    }
}

// Create generic classify function, which will be reused for the different types of events.
const classify = async (text) => {
    // Get the pipeline instance. This will load and build the model when run for the first time.
    let model = await PipelineSingleton.getInstance((data) => {
        // You can track the progress of the pipeline creation here.
        // e.g., you can send `data` back to the UI to indicate a progress bar
        // console.log('progress', data)
    });


Review comment (bug):
onnxruntime-web should have always deal with numThreads correctly. (ie. the default value should not fail its function) 

explicitly setting numThreads to 1 is no longer necessary.

Need help verification whether this is true.

Code after:
// background.js - Handles requests from the UI, runs the model, then sends back a response

import { pipeline, env } from '@xenova/transformers';

// Skip initial check for local models, since we are not loading any local models.
env.allowLocalModels = false;

// Due to a bug in onnxruntime-web, we must disable multithreading for now.
// See https://github.com/microsoft/onnxruntime/issues/14445 for more information.
env.backends.onnx.wasm.numThreads = 1;


class PipelineSingleton {
    static task = 'text-classification';
    static model = 'Xenova/distilbert-base-uncased-finetuned-sst-2-english';
    static instance = null;

    static async getInstance(progress_callback = null) {
        if (this.instance === null) {
            this.instance = pipeline(this.task, this.model, { progress_callback });
        }

        return this.instance;
    }
}

// Create generic classify function, which will be reused for the different types of events.
const classify = async (text) => {
    // Get the pipeline instance. This will load and build the model when run for the first time.
    let model = await PipelineSingleton.getInstance((data) => {
        // You can track the progress of the pipeline creation here.
        // e.g., you can send `data` back to the UI to indicate a progress bar
        // console.log('progress', data)
    });
Repository: xenova/transformers.js
File: webpack.config.js (JavaScript)

Code before:
          // Do not bundle with comments.
          // See https://webpack.js.org/plugins/terser-webpack-plugin/#remove-comments for more information.
          terserOptions: {
            output: {
              comments: false,
            },
          },
          extractComments: false,
        }),
      ],
    },
    experiments: {
      outputModule,
    },
    resolve: { alias },

    externals: externalModules,

    // Development server
    devServer: {
      static: {
        directory: __dirname,
      },
      port: 8080,
    },
  };

  if (outputModule) {
    // config.module = {
    //   parser: {
    //     javascript: {
    //       importMeta: false,
    //     },
    //   },
    // };
  } else {
    config.externalsType = "commonjs";
  }

  return config;
}

// Do not bundle onnxruntime-web when packaging for Node.js.
// Instead, we use the native library (onnxruntime-node).
const NODE_IGNORE_MODULES = ["onnxruntime-web"];

// Do not bundle the following modules with webpack (mark as external)
// NOTE: This is necessary for both type="module" and type="commonjs",
// and will be ignored when building for web (only used for node/deno)
const NODE_EXTERNAL_MODULES = [
  "onnxruntime-node",

Review comment (suggestion):
This is why the final bundle includes `new URL("./", import.meta.url)`.

Setting it to false to disable the chunks feature of webpack (which is not used anyway)

Code after:
  ignoreModules = [],
  externalModules = [],
  plugins = [],
} = {}) {
  const outputModule = type === "module";

  const alias = Object.fromEntries(
    ignoreModules.map((module) => [module, false]),
  );

  /** @type {import('webpack').Configuration} */
  const config = {
    mode: "development",
    devtool: "source-map",
    entry: {
      [`transformers${name}`]: "./src/transformers.js",
      [`transformers${name}.min`]: "./src/transformers.js",
    },
    output: {
      filename: `[name]${suffix}`,
      path: path.join(__dirname, "dist"),
      library: {
        type,
      },
      assetModuleFilename: "[name][ext]",
      chunkFormat: false,
    },
    optimization: {
      minimize: true,
      minimizer: [
        new TerserPlugin({
          test: new RegExp(`\\.min\\${suffix}$`),

          // Do not bundle with comments.
          // See https://webpack.js.org/plugins/terser-webpack-plugin/#remove-comments for more information.
          terserOptions: {
            output: {
              comments: false,
            },
          },
          extractComments: false,
        }),
      ],
    },
    experiments: {
      outputModule,
    },
    resolve: { alias },

    externals: externalModules,
Repository: xvzc/SpoofDPI
File: dns/doh.go (Go)

Code before:
type DOHClient struct {
	upstream string
	client   *http.Client
}

var client *DOHClient
var clientOnce sync.Once

func getDOHClient(host string) *DOHClient {
	clientOnce.Do(func() {
		if client == nil {
			c := &http.Client{
				Timeout: 5 * time.Second,
				Transport: &http.Transport{
					DialContext: (&net.Dialer{
						Timeout:   3 * time.Second,
						KeepAlive: 30 * time.Second,
					}).DialContext,
					TLSHandshakeTimeout: 5 * time.Second,
					MaxIdleConnsPerHost: 100,
					MaxIdleConns:        100,
				},
			}

			client = &DOHClient{
				upstream: "https://" + host + "/dns-query",
				client:   c,
			}
		}
	})

	return client
}

func (d *DOHClient) dohQuery(ctx context.Context, msg *dns.Msg) (*dns.Msg, error) {
	pack, err := msg.Pack()
	if err != nil {
		return nil, err
	}

	url := fmt.Sprintf("%s?dns=%s", d.upstream, base64.RawStdEncoding.EncodeToString(pack))
	req, err := http.NewRequest("GET", url, nil)
	if err != nil {
		return nil, err
	}

	req = req.WithContext(ctx)
	req.Header.Set("Accept", "application/dns-message")

	resp, err := d.client.Do(req)
	if err != nil {

Review comment (suggestion):
This might be a bad idea. Since DoH providers do this (not a flippin' standard somehow, grrrrrrrr):
<img width="238" alt="image" src="https://github.com/user-attachments/assets/faf3862d-1714-4d44-9728-063479991379">
<img width="498" alt="image" src="https://github.com/user-attachments/assets/a2955720-4771-45cf-af48-8b52c9048e6e">

Making the user guess that they need to strip the URL might not be intuitive

Code after:

type DOHClient struct {
	upstream string
	httpClient   *http.Client
}

var dohClient *DOHClient
var clientOnce sync.Once

func getDOHClient(host string) *DOHClient {
	if dohClient != nil {
		return dohClient
	}

	clientOnce.Do(func() {
		h := &http.Client{
			Timeout: 5 * time.Second,
			Transport: &http.Transport{
				DialContext: (&net.Dialer{
					Timeout:   3 * time.Second,
					KeepAlive: 30 * time.Second,
				}).DialContext,
				TLSHandshakeTimeout: 5 * time.Second,
				MaxIdleConnsPerHost: 100,
				MaxIdleConns:        100,
			},
		}

		host = regexp.MustCompile(`^https:\/\/|\/dns-query$`).ReplaceAllString(host, "")
		dohClient = &DOHClient{
			upstream: "https://" + host + "/dns-query",
			httpClient:   h,
		}
	})

	return dohClient
}

func (d *DOHClient) dohQuery(ctx context.Context, msg *dns.Msg) (*dns.Msg, error) {
	pack, err := msg.Pack()
	if err != nil {
		return nil, err
	}

	url := fmt.Sprintf("%s?dns=%s", d.upstream, base64.RawStdEncoding.EncodeToString(pack))
	req, err := http.NewRequest("GET", url, nil)
	if err != nil {
		return nil, err
	}

	req = req.WithContext(ctx)
Repository: zellij-org/zellij
File: src/utils/logging.rs (Rust)

Code before:
use std::{
    fs,
    io::{self, prelude::*},
    os::unix::io::RawFd,
    path::PathBuf,
};

use crate::utils::consts::*;

pub fn debug_log_to_file(message: String) -> io::Result<()> {
    let mut file = fs::OpenOptions::new()
        .append(true)
        .create(true)
        .open(MOSAIC_TMP_LOG_FILE)?;
    file.write_all(message.as_bytes())?;
    file.write_all("\n".as_bytes())?;

    Ok(())
}

pub fn debug_log_to_file_pid_0(message: String, pid: RawFd) -> io::Result<()> {
    if pid == 0 {
        debug_log_to_file(message)?;
    }

    Ok(())
}

pub fn delete_log_file() -> io::Result<()> {
    if fs::metadata(MOSAIC_TMP_LOG_FILE).is_ok() {
        fs::remove_file(MOSAIC_TMP_LOG_FILE)?;
    }


Review comment (suggestion):
I actually very much prefer to be explicit about imports. I think it makes it much clearer when you're trying to figure out where something is coming from.

Code after:
use std::{
    fs,
    io::{self, prelude::*},
    os::unix::io::RawFd,
    path::PathBuf,
};

use crate::utils::consts::{MOSAIC_TMP_LOG_DIR, MOSAIC_TMP_LOG_FILE};

pub fn debug_log_to_file(message: String) -> io::Result<()> {
    let mut file = fs::OpenOptions::new()
        .append(true)
        .create(true)
        .open(MOSAIC_TMP_LOG_FILE)?;
    file.write_all(message.as_bytes())?;
    file.write_all("\n".as_bytes())?;

    Ok(())
}

pub fn debug_log_to_file_pid_0(message: String, pid: RawFd) -> io::Result<()> {
    if pid == 0 {
        debug_log_to_file(message)?;
    }

    Ok(())
}

pub fn delete_log_file() -> io::Result<()> {
    if fs::metadata(MOSAIC_TMP_LOG_FILE).is_ok() {
        fs::remove_file(MOSAIC_TMP_LOG_FILE)?;
    }
Repository: zellij-org/zellij
File: src/utils/logging.rs (Rust)

Code before:

use crate::utils::consts::{MOSAIC_TMP_LOG_DIR, MOSAIC_TMP_LOG_FILE};

fn atomic_create_file(file_name: &str) {
    let _ = fs::OpenOptions::new().create(true).open(file_name);
}

pub fn debug_log_to_file(message: String) -> io::Result<()> {
    atomic_create_file(MOSAIC_TMP_LOG_FILE);
    let mut file = fs::OpenOptions::new()
        .append(true)
        .create(true)
        .open(MOSAIC_TMP_LOG_FILE)?;
    file.write_all(message.as_bytes())?;
    file.write_all(b"\n")
}

pub fn debug_log_to_file_pid_0(message: String, pid: RawFd) -> io::Result<()> {
    if pid == 0 {
        debug_log_to_file(message)
    } else {
        Ok(())
    }
}

pub fn _delete_log_file() -> io::Result<()> {
    if fs::metadata(MOSAIC_TMP_LOG_FILE).is_ok() {
        fs::remove_file(MOSAIC_TMP_LOG_FILE)
    } else {
        Ok(())
    }
}

pub fn _delete_log_dir() -> io::Result<()> {
    if fs::metadata(MOSAIC_TMP_LOG_DIR).is_ok() {
        fs::remove_dir_all(MOSAIC_TMP_LOG_DIR)
    } else {
        Ok(())
    }
}

pub fn debug_to_file(message: u8, pid: RawFd) -> io::Result<()> {
    let mut path = PathBuf::new();
    path.push(MOSAIC_TMP_LOG_DIR);
    path.push(format!("mosaic-{}.log", pid.to_string()));

    let mut file = fs::OpenOptions::new()
        .append(true)
        .create(true)
        .open(path)?;
    file.write_all(&[message])

Review comment (bug):
The functions in this file are general utility functions. Can we add exceptions for them so that we don't have to prefix them with underscores? It would just be confusing and annoying while debugging :)

Code after:

use crate::utils::consts::{MOSAIC_TMP_LOG_DIR, MOSAIC_TMP_LOG_FILE};

pub fn atomic_create_file(file_name: &str) {
    let _ = fs::OpenOptions::new().create(true).open(file_name);
}

pub fn atomic_create_dir(dir_name: &str) -> io::Result<()> {
    if let Err(e) = fs::create_dir(dir_name) {
        if e.kind() == std::io::ErrorKind::AlreadyExists {
            Ok(())
        } else {
            Err(e)
        }
    } else {
        Ok(())
    }
}

pub fn debug_log_to_file(message: String) -> io::Result<()> {
    atomic_create_file(MOSAIC_TMP_LOG_FILE);
    let mut file = fs::OpenOptions::new()
        .append(true)
        .create(true)
        .open(MOSAIC_TMP_LOG_FILE)?;
    file.write_all(message.as_bytes())?;
    file.write_all(b"\n")
}

pub fn debug_log_to_file_pid_0(message: String, pid: RawFd) -> io::Result<()> {
    if pid == 0 {
        debug_log_to_file(message)
    } else {
        Ok(())
    }
}

#[allow(dead_code)]
pub fn delete_log_file() -> io::Result<()> {
    if fs::metadata(MOSAIC_TMP_LOG_FILE).is_ok() {
        fs::remove_file(MOSAIC_TMP_LOG_FILE)
    } else {
        Ok(())
    }
}

#[allow(dead_code)]
pub fn delete_log_dir() -> io::Result<()> {
    if fs::metadata(MOSAIC_TMP_LOG_DIR).is_ok() {
        fs::remove_dir_all(MOSAIC_TMP_LOG_DIR)
    } else {
Repository: zellij-org/zellij
File: src/common/screen.rs (Rust)

Code before:
    pub fn update_active_tab_name(&mut self, buf: Vec<u8>) {
        let s = str::from_utf8(&buf).unwrap();
        match s {
            "\0" => {
                self.tabname_buf = String::new();
            }
            "\n" => {
                let new_name = self.tabname_buf.clone();
                let active_tab = self.get_active_tab_mut().unwrap();
                active_tab.name = new_name;
                self.update_tabs();
                self.render();
            }
            "\u{007F}" | "\u{0008}" => {
                //delete and backspace keys
                self.tabname_buf.pop();
            }
            c => {
                self.tabname_buf.push_str(c);
            }
        }
    }
    pub fn change_input_mode(&mut self, input_mode: InputMode) {
        self.input_mode = input_mode;
        self.get_active_tab_mut().unwrap().input_mode = self.input_mode;
        self.render();
    }
}


Review comment (refactor):
In the input handler where we send the `ChangeInputMode` instruction, we already explicitly send a `Render` instruction later... maybe we can remove this extra render then?

Code after:
        let s = str::from_utf8(&buf).unwrap();
        match s {
            "\0" => {
                self.tabname_buf = String::new();
            }
            "\n" => {
                let new_name = self.tabname_buf.clone();
                let active_tab = self.get_active_tab_mut().unwrap();
                active_tab.name = new_name;
                self.update_tabs();
                self.render();
            }
            "\u{007F}" | "\u{0008}" => {
                //delete and backspace keys
                self.tabname_buf.pop();
            }
            c => {
                self.tabname_buf.push_str(c);
            }
        }
    }
    pub fn change_input_mode(&mut self, input_mode: InputMode) {
        self.input_mode = input_mode;
        for tab in self.tabs.values_mut() {
            tab.input_mode = self.input_mode;
        }
    }
}
Repository: zellij-org/zellij
File: src/client/panes/plugin_pane.rs (Rust)

Code before:
use crate::{common::SenderWithContext, pty_bus::VteBytes, tab::Pane, wasm_vm::PluginInstruction};

use std::{sync::mpsc::channel, unimplemented};

use crate::panes::{PaneId, PositionAndSize};
use crate::utils::shared::secs_from_epoch;

pub struct PluginPane {
    pub pid: u32,
    pub should_render: bool,
    pub selectable: bool,
    pub invisible_borders: bool,
    pub position_and_size: PositionAndSize,
    pub position_and_size_override: Option<PositionAndSize>,
    pub send_plugin_instructions: SenderWithContext<PluginInstruction>,
    pub max_height: Option<usize>,
    pub active_at: u64,
}

impl PluginPane {
    pub fn new(
        pid: u32,
        position_and_size: PositionAndSize,
        send_plugin_instructions: SenderWithContext<PluginInstruction>,
    ) -> Self {
        Self {
            pid,
            should_render: true,
            selectable: true,
            invisible_borders: false,
            position_and_size,
            position_and_size_override: None,
            send_plugin_instructions,
            max_height: None,
            active_at: secs_from_epoch(),
        }
    }
}

impl Pane for PluginPane {
    // FIXME: These position and size things should all be moved to default trait implementations,
    // with something like a get_pos_and_sz() method underpinning all of them. Alternatively and

Review comment (suggestion):
As a small code tweak though, I'd prefer if we used [`Instant`](https://doc.rust-lang.org/std/time/struct.Instant.html)s here. Those are actually guaranteed to never do anything odd around leap-seconds and are much more idiomatic Rust (just in my opinion) 

Code after:
use crate::{common::SenderWithContext, pty_bus::VteBytes, tab::Pane, wasm_vm::PluginInstruction};

use crate::panes::{PaneId, PositionAndSize};

use std::time::Instant;
use std::{sync::mpsc::channel, unimplemented};

pub struct PluginPane {
    pub pid: u32,
    pub should_render: bool,
    pub selectable: bool,
    pub invisible_borders: bool,
    pub position_and_size: PositionAndSize,
    pub position_and_size_override: Option<PositionAndSize>,
    pub send_plugin_instructions: SenderWithContext<PluginInstruction>,
    pub max_height: Option<usize>,
    pub max_width: Option<usize>,
    pub active_at: Instant,
}

impl PluginPane {
    pub fn new(
        pid: u32,
        position_and_size: PositionAndSize,
        send_plugin_instructions: SenderWithContext<PluginInstruction>,
    ) -> Self {
        Self {
            pid,
            should_render: true,
            selectable: true,
            invisible_borders: false,
            position_and_size,
            position_and_size_override: None,
            send_plugin_instructions,
            max_height: None,
            max_width: None,
            active_at: Instant::now(),
        }
    }
}

impl Pane for PluginPane {
Repository: zellij-org/zellij
File: zellij-utils/src/input/options.rs (Rust)

Code before:
//! Handles cli and configuration options
use crate::cli::Command;
use serde::{Deserialize, Serialize};
use std::path::PathBuf;
use structopt::StructOpt;
use zellij_tile::data::InputMode;

#[derive(Clone, Default, Debug, PartialEq, Deserialize, Serialize, StructOpt)]
/// Options that can be set either through the config file,
/// or cli flags
pub struct Options {
    /// Allow plugins to use a more simplified layout
    /// that is compatible with more fonts
    #[structopt(long)]
    #[serde(default)]
    pub simplified_ui: bool,
    /// Set the default theme
    #[structopt(long)]
    pub theme: Option<String>,
    /// Set the default mode
    #[structopt(long)]
    pub default_mode: Option<InputMode>,
    /// Set the default shell
    #[structopt(long, parse(from_os_str))]
    pub default_shell: Option<PathBuf>,
    /// Set the layout_dir, defaults to
    /// subdirectory of config dir
    #[structopt(long, parse(from_os_str))]
    pub layout_dir: Option<PathBuf>,
    #[structopt(long)]
    #[serde(default)]
    pub disable_mouse_mode: bool,
}


Review comment (suggestion):
This looks good. I am unsure if this is the correct place for it,
but the actions file should probably be leaner than this one, 
so maybe this is the correct place. You definitely didn't do
unnecessary stuff here.

Code after:
//! Handles cli and configuration options
use crate::cli::Command;
use serde::{Deserialize, Serialize};
use std::path::PathBuf;
use std::str::FromStr;
use structopt::StructOpt;
use zellij_tile::data::InputMode;

#[derive(Copy, Clone, Debug, PartialEq, Deserialize, Serialize)]
pub enum OnForceClose {
    Quit,
    Detach,
}

impl Default for OnForceClose {
    fn default() -> Self {
        Self::Detach
    }
}

impl FromStr for OnForceClose {
    type Err = Box<dyn std::error::Error>;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "quit" => Ok(Self::Quit),
            "detach" => Ok(Self::Detach),
            e => Err(e.to_string().into()),
        }
    }
}

#[derive(Clone, Default, Debug, PartialEq, Deserialize, Serialize, StructOpt)]
/// Options that can be set either through the config file,