ronantakizawa/codereview-bench huggingface dataset:
Repository: TabbyML/tabby
File: ee/tabby-webserver/src/service/background_job/db.rs (Rust)

Code before:

impl DbMaintainanceJob {
    pub async fn cron(
        _now: DateTime<Utc>,
        context: Arc<dyn ContextService>,
        db: DbConn,
    ) -> tabby_schema::Result<()> {
        db.delete_expired_token().await?;
        db.delete_expired_password_resets().await?;
        db.delete_expired_ephemeral_threads().await?;

        // Read all active sources
        let active_source_ids = context
            .read(None)
            .await?
            .sources
            .into_iter()
            .map(|x| x.source_id())
            .collect::<Vec<_>>();

        db.delete_unused_source_id_read_access_policy(&active_source_ids)
            .await?;
        Ok(())
    }

    pub async fn retention(now: DateTime<Utc>, db: DbConn) -> tabby_schema::Result<()> {
        if let Some(three_months_ago) = now.checked_sub_months(Months::new(3)) {
            if let Err(e) = db.delete_jobs_before(three_months_ago).await {
                error!(
                    "Failed to clean up and retain only the last 3 months of jobs: {:?}",
                    e
                );
            }

            if let Err(e) = db.delete_user_events_before(three_months_ago).await {
                error!(
                    "Failed to clean up and retain only the last 3 months of user events: {:?}",
                    e
                );
            }
        };
        Ok(())
    }
}

#[cfg(test)]
mod tests {
    use chrono::{DateTime, Utc};
    use tabby_db::DbConn;

    use super::*;

Review comment (suggestion):
```suggestion
    async fn data_retention(now: DateTime<Utc>, db: DbConn) -> tabby_schema::Result<()> {
```

just call it in `cron`

Code after:

impl DbMaintainanceJob {
    pub async fn cron(
        now: DateTime<Utc>,
        context: Arc<dyn ContextService>,
        db: DbConn,
    ) -> tabby_schema::Result<()> {
        db.delete_expired_token().await?;
        db.delete_expired_password_resets().await?;
        db.delete_expired_ephemeral_threads().await?;

        // Read all active sources
        let active_source_ids = context
            .read(None)
            .await?
            .sources
            .into_iter()
            .map(|x| x.source_id())
            .collect::<Vec<_>>();

        db.delete_unused_source_id_read_access_policy(&active_source_ids)
            .await?;

        Self::data_retention(now, &db).await?;
        Ok(())
    }

    async fn data_retention(now: DateTime<Utc>, db: &DbConn) -> tabby_schema::Result<()> {
        db.delete_job_run_before_three_months(now)
            .await
            .context("Failed to clean up and retain only the last 3 months of jobs")?;

        db.delete_user_events_before_three_months(now)
            .await
            .context("Failed to clean up and retain only the last 3 months of user events")?;

        Ok(())
    }
}

#[cfg(test)]
mod tests {
    use chrono::{DateTime, Utc};
    use tabby_db::DbConn;

    use super::*;

    #[tokio::test]
    async fn test_retention_should_delete() {
        let db = DbConn::new_in_memory().await.unwrap();
        let cases = vec![
Repository: TabbyML/tabby
File: crates/tabby-common/src/config.rs (Rust)

Code before:
    #[serde(default)]
    pub rate_limit: RateLimit,

    /// Used by OpenAI style API for model name.
    #[builder(default)]
    pub model_name: Option<String>,

    /// Used by Completion API to construct a completion model.
    #[builder(default)]
    pub prompt_template: Option<String>,

    /// Used by Completion API to construct a chat model.
    #[builder(default)]
    pub chat_template: Option<String>,

    /// Used by Chat/Completion API allowing users to get supported models info.
    #[builder(default)]
    pub supported_models: Option<Vec<String>>,

    #[builder(default)]
    pub additional_stop_words: Option<Vec<String>>,

    /// Used For Azure API to specify the api version
    #[builder(default)]
    #[serde(default)]
    pub api_version: Option<String>,
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct LocalModelConfig {
    pub model_id: String,

    #[serde(default = "default_parallelism")]
    pub parallelism: u8,

    #[serde(default = "default_num_gpu_layers")]
    pub num_gpu_layers: u16,

    #[serde(default)]
    pub enable_fast_attention: Option<bool>,

    #[serde(default = "default_context_size")]
    pub context_size: usize,

    #[serde(default)]
    pub additional_stop_words: Option<Vec<String>>,
}

fn default_parallelism() -> u8 {
    1
}

Review comment (suggestion):
```suggestion
```

Let's keep the config unchanged and set the api version to a static value.

Code after:
    #[serde(default)]
    pub rate_limit: RateLimit,

    /// Used by OpenAI style API for model name.
    #[builder(default)]
    pub model_name: Option<String>,

    /// Used by Completion API to construct a completion model.
    #[builder(default)]
    pub prompt_template: Option<String>,

    /// Used by Completion API to construct a chat model.
    #[builder(default)]
    pub chat_template: Option<String>,

    /// Used by Chat/Completion API allowing users to get supported models info.
    #[builder(default)]
    pub supported_models: Option<Vec<String>>,

    #[builder(default)]
    pub additional_stop_words: Option<Vec<String>>,
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct LocalModelConfig {
    pub model_id: String,

    #[serde(default = "default_parallelism")]
    pub parallelism: u8,

    #[serde(default = "default_num_gpu_layers")]
    pub num_gpu_layers: u16,

    #[serde(default)]
    pub enable_fast_attention: Option<bool>,

    #[serde(default = "default_context_size")]
    pub context_size: usize,

    #[serde(default)]
    pub additional_stop_words: Option<Vec<String>>,
}

fn default_parallelism() -> u8 {
    1
}

fn default_num_gpu_layers() -> u16 {
    9999
}
Repository: TabbyML/tabby
File: clients/tabby-agent/src/codeLens.ts (TypeScript)

Code before:
      originLines,
      originCodeLenses.map((item) => item.range),
      modifiedLines,
      modifiedCodeLenses.map((item) => item.range),
    );
    const deletionDecorations = originRanges.map((range) => {
      return {
        range,
        data: {
          type: codeLensType,
          text: "deleted" as const,
        },
      };
    });

    const insertionDecorations = modifiedRanges.map((range) => {
      return {
        range,
        data: {
          type: codeLensType,
          text: "inserted" as const,
        },
      };
    });

    codeLenses.push(...deletionDecorations, ...insertionDecorations);
    logger.debug(`codeLenses: ${JSON.stringify(codeLenses)}`);

    workDoneProgress?.done();
    if (resultProgress) {
      return null;
    } else {
      return codeLenses;
    }
  }
}


Review comment (suggestion):
```suggestion
    if (resultProgress) {
      resultProgress.report([...deletionDecorations, ...insertionDecorations]);
    } else {
      codeLenses.push(...deletionDecorations, ...insertionDecorations);
    }
```

Code after:
      originLines,
      originCodeLenses.map((item) => item.range),
      modifiedLines,
      modifiedCodeLenses.map((item) => item.range),
    );
    const deletionDecorations = originRanges.map((range) => {
      return {
        range,
        data: {
          type: codeLensType,
          text: "deleted" as const,
        },
      };
    });

    const insertionDecorations = modifiedRanges.map((range) => {
      return {
        range,
        data: {
          type: codeLensType,
          text: "inserted" as const,
        },
      };
    });

    if (resultProgress) {
      resultProgress.report([...deletionDecorations, ...insertionDecorations]);
    } else {
      codeLenses.push(...deletionDecorations, ...insertionDecorations);
    }
    logger.debug(`codeLenses: ${JSON.stringify(codeLenses)}`);

    workDoneProgress?.done();
    if (resultProgress) {
      return null;
    } else {
      return codeLenses;
    }
  }
}
Repository: TabbyML/tabby
File: ee/tabby-schema/src/schema/page/types.rs (Rust)

Code before:
}

#[derive(GraphQLObject)]
pub struct PageSectionContentDelta {
    pub id: ID,
    pub delta: String,
}

#[derive(GraphQLObject)]
pub struct PageSectionContentCompleted {
    pub id: ID,
}

/// Schema of page convert stream.
#[derive(GraphQLUnion)]
#[graphql(context = Context)]
pub enum PageConvertItem {
    // PageCreated will return at the beginning of the stream,
    // containing the page ID, author and title.
    PageCreated(PageCreated),

    PageContentDelta(PageContentDelta),
    PageContentCompleted(PageContentCompleted),

    // PageSectionsTitleCreated will return the titles of all sections.
    PageSectionsTitleCreated(PageSectionsTitleCreated),

    PageSectionContentDelta(PageSectionContentDelta),
    PageSectionContentCompleted(PageSectionContentCompleted),
}


Review comment (suggestion):
```suggestion
    PageSectionsCreated(PageSectionsCreated),
```

Code after:
}

#[derive(GraphQLObject)]
pub struct PageSectionContentDelta {
    pub id: ID,
    pub delta: String,
}

#[derive(GraphQLObject)]
pub struct PageSectionContentCompleted {
    pub id: ID,
}

/// Schema of page convert stream.
#[derive(GraphQLUnion)]
#[graphql(context = Context)]
pub enum PageRunItem {
    // PageCreated will return at the beginning of the stream,
    // containing the page ID, author and title.
    PageCreated(PageCreated),

    PageContentDelta(PageContentDelta),
    PageContentCompleted(PageContentCompleted),

    // PageSectionsCreated will return the titles of all sections.
    PageSectionsCreated(PageSectionsCreated),

    PageSectionContentDelta(PageSectionContentDelta),
    PageSectionContentCompleted(PageSectionContentCompleted),
}
Repository: TabbyML/tabby
File: crates/tabby-git/src/lib.rs (Rust)

Code before:

mod grep;
use std::path::Path;

use axum::{
    body::Body,
    http::{Response, StatusCode},
};
use file_search::GitFileSearch;
use futures::Stream;
pub use grep::{GrepFile, GrepLine, GrepSubMatch, GrepTextOrBase64};

pub async fn search_files(
    root: &Path,
    rev: Option<&str>,
    pattern: &str,
    limit: usize,
) -> anyhow::Result<Vec<GitFileSearch>> {
    file_search::search(git2::Repository::open(root)?, rev, pattern, limit).await
}

pub async fn list_files(
    root: &Path,
    rev: Option<&str>,
    limit: Option<usize>,
) -> anyhow::Result<(Vec<GitFileSearch>, bool)> {
    file_search::list(git2::Repository::open(root)?, rev, limit).await
}

pub async fn grep(
    root: &Path,
    rev: Option<&str>,
    query: &str,
) -> anyhow::Result<impl Stream<Item = GrepFile>> {
    let repository = git2::Repository::open(root)?;
    let query: grep::GrepQuery = query.parse()?;
    grep::grep(repository, rev, &query)
}

pub fn serve_file(
    root: &Path,
    commit: Option<&str>,
    path: Option<&str>,
) -> std::result::Result<Response<Body>, StatusCode> {
    let repository = git2::Repository::open(root).map_err(|_| StatusCode::NOT_FOUND)?;
    serve_git::serve(&repository, commit, path)
}

pub struct GitReference {
    pub name: String,
    pub commit: String,

Review comment (suggestion):
define a struct, e.g
```
struct ListFile {
  files: Vec<>,
  truncated: bool
}
```

Code after:

mod grep;
use std::path::Path;

use axum::{
    body::Body,
    http::{Response, StatusCode},
};
use file_search::GitFileSearch;
use futures::Stream;
pub use grep::{GrepFile, GrepLine, GrepSubMatch, GrepTextOrBase64};

pub async fn search_files(
    root: &Path,
    rev: Option<&str>,
    pattern: &str,
    limit: usize,
) -> anyhow::Result<Vec<GitFileSearch>> {
    file_search::search(git2::Repository::open(root)?, rev, pattern, limit).await
}

pub struct ListFile {
    pub files: Vec<GitFileSearch>,
    pub truncated: bool,
}

pub async fn list_files(
    root: &Path,
    rev: Option<&str>,
    limit: Option<usize>,
) -> anyhow::Result<ListFile> {
    let (files, truncated) = file_search::list(git2::Repository::open(root)?, rev, limit).await?;
    Ok(ListFile { files, truncated })
}

pub async fn grep(
    root: &Path,
    rev: Option<&str>,
    query: &str,
) -> anyhow::Result<impl Stream<Item = GrepFile>> {
    let repository = git2::Repository::open(root)?;
    let query: grep::GrepQuery = query.parse()?;
    grep::grep(repository, rev, &query)
}

pub fn serve_file(
    root: &Path,
    commit: Option<&str>,
    path: Option<&str>,
) -> std::result::Result<Response<Body>, StatusCode> {
    let repository = git2::Repository::open(root).map_err(|_| StatusCode::NOT_FOUND)?;
Repository: TabbyML/tabby
File: clients/vscode/src/keybindings.ts (TypeScript)

Code before:
import { readFile } from "fs-extra";
import path from "path";
import { getLogger } from "./logger";
import * as vscode from "vscode";
import { isBrowser } from "./env";
import pkg from "../package.json";
interface KeyBinding {
  key: string;
  command: string;
  when?: string;
}

const logger = getLogger("VSCodeKeyBindingManager");
const isMac = isBrowser
  ? navigator.userAgent.toLowerCase().includes("mac")
  : process.platform.toLowerCase().includes("darwin");

export class VSCodeKeyBindingManager {
  // Singleton instance to manage keybindings.
  private static instance: VSCodeKeyBindingManager | null = null;

  /**
   * Returns the singleton instance.
   */
  public static getInstance(): VSCodeKeyBindingManager {
    if (!VSCodeKeyBindingManager.instance) {
      VSCodeKeyBindingManager.instance = new VSCodeKeyBindingManager();
    }
    return VSCodeKeyBindingManager.instance;
  }

  // Cached keybindings loaded during extension startup.
  private keybindings: KeyBinding[] | null = null;

  /**
   * Initializes the keybinding manager.
   * This method should be called once during extension startup.
   * It reads the keybindings.json file once to avoid additional overhead.
   */
  public async init(): Promise<void> {
    this.keybindings = await this.readKeyBindings();
  }


Review comment (suggestion):
```suggestion
export class KeyBindingManager {
```

Code after:
import { readFile } from "fs-extra";
import path from "path";
import { getLogger } from "./logger";
import * as vscode from "vscode";
import { isBrowser } from "./env";
import pkg from "../package.json";
interface KeyBinding {
  key: string;
  command: string;
  when?: string;
}

const logger = getLogger("VSCodeKeyBindingManager");
const isMac = isBrowser
  ? navigator.userAgent.toLowerCase().includes("mac")
  : process.platform.toLowerCase().includes("darwin");

export class KeyBindingManager {
  // Singleton instance to manage keybindings.
  private static instance: KeyBindingManager | null = null;

  /**
   * Returns the singleton instance.
   */
  public static getInstance(): KeyBindingManager {
    if (!KeyBindingManager.instance) {
      KeyBindingManager.instance = new KeyBindingManager();
    }
    return KeyBindingManager.instance;
  }

  // Cached keybindings loaded during extension startup.
  private keybindings: KeyBinding[] | null = null;

  /**
   * Initializes the keybinding manager.
   * This method should be called once during extension startup.
   * It reads the keybindings.json file once to avoid additional overhead.
   */
  public async init(): Promise<void> {
    this.keybindings = await this.readKeyBindings();
  }
Repository: TabbyML/tabby
File: ee/tabby-webserver/src/service/utils/prompt.rs (Rust)

Code before:
        .as_deref()
        .ok_or_else(|| anyhow!("Failed to get content from chat completion"))?;

    Ok(content.into())
}

/// Extracts items from the given content.
/// Assumptions:
/// 1. Each item is on a new line.
/// 2. Items may start with a bullet point, which needs to be trimmed.
pub fn transform_line_items(content: &str) -> Vec<String> {
    content
        .lines()
        .map(trim_bullet)
        .filter(|x| !x.is_empty())
        .map(|x| x.to_owned())
        .collect()
}

/// Trims leading and trailing bullet-like characters or digits from the provided string and returns the trimmed result.
fn trim_bullet(s: &str) -> &str {
    let s = s.trim();
    if s.starts_with('-') || s.starts_with('+') || s.starts_with('*') {
        return s.trim_matches(&['-', '+', '*', ' '][..]);
    }

    // Check for numbered list
    if let Some(pos) = s.find(". ") {
        let prefix = &s[..pos];
        if prefix.chars().all(|c| c.is_numeric()) {
            return trim_bullet(s[pos + 2..].trim_start());
        }
    }

    s
}

pub fn trim_title(title: &str) -> &str {
    // take first line.
    let title = title.lines().next().unwrap_or(title).trim();
    trim_bullet(title.trim_matches(&['"', '#', ' ', '-', '*'][..]))
}

/// Checks if the `check` string is contained within `content` in a case-insensitive manner.
pub fn detect_content(content: &str, check: &str) -> bool {
    content.to_lowercase().contains(check)
}

#[cfg(test)]
mod tests {
    use super::*;

Review comment (suggestion):
```suggestion
let s =  s.trim().s.trim_matches(&['-', '+', '*', ' ']).trim()
```

Code after:
        .as_deref()
        .ok_or_else(|| anyhow!("Failed to get content from chat completion"))?;

    Ok(content.into())
}

/// Extracts items from the given content.
/// Assumptions:
/// 1. Each item is on a new line.
/// 2. Items may start with a bullet point, which needs to be trimmed.
pub fn transform_line_items(content: &str) -> Vec<String> {
    content
        .lines()
        .map(trim_bullet)
        .filter(|x| !x.is_empty())
        .map(|x| x.to_owned())
        .collect()
}

/// Trims leading and trailing bullet-like characters or digits from the provided string and returns the trimmed result.
fn trim_bullet(s: &str) -> &str {
    let s = s.trim().trim_matches(['-', '+', '*', ' ']).trim();

    // Check for numbered list, like "1. Hello", "12. Hello"
    if let Some(pos) = s.find(". ") {
        let prefix = &s[..pos];
        if prefix.chars().all(|c| c.is_numeric()) {
            return trim_bullet(s[pos + 2..].trim_start());
        }
    }

    s
}

pub fn trim_title(title: &str) -> &str {
    // take first line.
    let title = title.lines().next().unwrap_or(title).trim();
    trim_bullet(title.trim_matches(&['"', '#'][..]))
}

/// Checks if the `check` string is contained within `content` in a case-insensitive manner.
pub fn detect_content(content: &str, check: &str) -> bool {
    content.to_lowercase().contains(check)
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_trim_bullet() {
Repository: TabbyML/tabby
File: clients/vscode/src/extension.ts (TypeScript)

Code before:
  logger.info("Tabby extension activated.");

  // Start async initialization
  const startClient = async () => {
    await gitProvider.init();

    logger.info("Launching language server tabby-agent...");
    await client.start();
    logger.info("Language server tabby-agent launched.");
  };

  await Promise.all([
    // start LSP client
    startClient(),

    // findFiles preheat
    initFindFiles(context),
  ]);

  const tryReadAuthenticationToken = async (): Promise<{ token: string | undefined }> => {
    const response = await window.showInformationMessage(
      `Would you like to share your Tabby's token with endpoint ${config.serverEndpoint}?`,
      { modal: true, detail: "This will share your token to third parties VSCode extension" },
      "Yes",
      "No",
    );

    if (response === "Yes") {
      return { token: config.serverRecords.get(config.serverEndpoint)?.token };
    }
    return { token: undefined };
  };
  return {
    tryReadAuthenticationToken,
  };
}

export async function deactivate() {
  logger.info("Deactivating Tabby extension...");
  await clientRef?.stop();
  logger.info("Tabby extension deactivated.");
}


Review comment (suggestion):
```suggestion
const response = await window.showInformationMessage(
  "Do you consent to sharing your Tabby token with another VSCode extension?",
  { 
    modal: true, 
    detail: `The extension requests your token to access the Tabby server at ${config.serverEndpoint}. Sharing your token allows the extension to perform actions as if it were you. Only proceed if you trust the extension.` 
  },
  "Yes",
  "No"
);
```

Code after:
  logger.info("Tabby extension activated.");

  // Start async initialization
  const startClient = async () => {
    await gitProvider.init();

    logger.info("Launching language server tabby-agent...");
    await client.start();
    logger.info("Language server tabby-agent launched.");
  };

  await Promise.all([
    // start LSP client
    startClient(),

    // findFiles preheat
    initFindFiles(context),
  ]);

  const tryReadAuthenticationToken = async (): Promise<{ token: string | undefined }> => {
    const response = await window.showInformationMessage(
      "Do you consent to sharing your Tabby token with another VSCode extension?",
      {
        modal: true,
        detail: `The extension requests your token to access the Tabby server at ${config.serverEndpoint}. Sharing your token allows the extension to perform actions as if it were you. Only proceed if you trust the extension.`,
      },
      "Yes",
      "No",
    );

    if (response === "Yes") {
      return { token: config.serverRecords.get(config.serverEndpoint)?.token };
    }
    return { token: undefined };
  };
  return {
    tryReadAuthenticationToken,
  };
}

export async function deactivate() {
  logger.info("Deactivating Tabby extension...");
  await clientRef?.stop();
  logger.info("Tabby extension deactivated.");
}
Repository: TabbyML/tabby
File: clients/tabby-chat-panel/src/client.ts (TypeScript)

Code before:
  /**
   * The symbol name.
   */
  label: string

  /**
   * The filepath of the containing file.
   */
  filepath: Filepath

  /**
   * The line range of the symbol definition in the file.
   */
  range: LineRange
}

/**
 * Parameters for retrieving Git changes from the client IDE.
 * Used with {@link ClientApi.getChanges} method.
 */
export interface GetChangesParams {
  /**
   * The maximum number of tokens to return in the change content.
   * If not provided, all changes will be returned without token limitation.
   */
  limitToken?: number
}
/**
 * Represents a Git change item returned by the {@link ClientApi.getChanges} method.
 * Contains information about modifications detected in the Git workspace.
 */
export interface ChangeItem {
  /**
   * The content of the change, formatted in the same Diff format as the `git diff` command.
   * This includes information about added, modified, and deleted lines in the affected files.
   */
  content: string
  /**
   * Indicates whether the change has been staged in the Git index.
   * True if the change is staged, false if it is unstaged.
   */
  staged: boolean
}


Review comment (suggestion):
```suggestion
  maxChars?: number
```
This parameter is used to limit the token count. However, the client cannot calculate the token count accurately, and approximate token count calculations should not be performed on the client side either.  
I prefer using `maxChars` here as it provides a precise definition.


Code after:
  limit?: number
}

/**
 * The item returned by {@link ClientApi.listSymbols}.
 */
export interface ListSymbolItem {
  /**
   * The symbol name.
   */
  label: string

  /**
   * The filepath of the containing file.
   */
  filepath: Filepath

  /**
   * The line range of the symbol definition in the file.
   */
  range: LineRange
}

/**
 * Parameters for retrieving Git changes from the client IDE.
 * Used with {@link ClientApi.getChanges} method.
 */
export interface GetChangesParams {
  /**
   * The maximum number of characters to return in the change content.
   * If not provided, all changes will be returned without character limitation.
   */
  maxChars?: number
}
/**
 * Represents a Git change item returned by the {@link ClientApi.getChanges} method.
 * Contains information about modifications detected in the Git workspace.
 */
export interface ChangeItem {
  /**
   * The content of the change, formatted in the same Diff format as the `git diff` command.
   * This includes information about added, modified, and deleted lines in the affected files.
   */
  content: string
  /**
   * Indicates whether the change has been staged in the Git index.
   * True if the change is staged, false if it is unstaged.
   */
  staged: boolean
}