ronantakizawa/codereview-bench huggingface dataset:
Repository: danny-avila/LibreChat
File: client/src/components/Chat/Menus/PresetsMenu.tsx (TypeScript)

Code before:
import type { FC } from 'react';
import { useRef, useEffect } from 'react';
import { BookCopy } from 'lucide-react';
import {
  Button,
  OGDialog,
  OGDialogTitle,
  OGDialogHeader,
  OGDialogContent,
  TooltipAnchor,
} from '@librechat/client';
import { Content, Portal, Root, Trigger } from '@radix-ui/react-popover';
import { EditPresetDialog, PresetItems } from './Presets';
import { useLocalize, usePresets } from '~/hooks';
import { useChatContext } from '~/Providers';

const PresetsMenu: FC = () => {
  const localize = useLocalize();
  const presetsMenuTriggerRef = useRef<HTMLDivElement>(null);
  const {
    presetsQuery,
    onSetDefaultPreset,
    onFileSelected,
    onSelectPreset,
    onChangePreset,
    clearAllPresets,
    onDeletePreset,
    submitPreset,
    exportPreset,
    showDeleteDialog,
    setShowDeleteDialog,
    presetToDelete,
    confirmDeletePreset,
  } = usePresets();
  const { preset } = useChatContext();

Review comment (suggestion):
TooltipAnchor should be listed by line length. 

`import { Content, Portal, Root, Trigger } from '@radix-ui/react-popover';` should precede the client imports. 

> import type { FC } from 'react';

should be after package-level imports

Code after:
import { useRef } from 'react';
import { BookCopy } from 'lucide-react';
import { Content, Portal, Root, Trigger } from '@radix-ui/react-popover';
import {
  Button,
  OGDialog,
  TooltipAnchor,
  OGDialogTitle,
  OGDialogHeader,
  OGDialogContent,
} from '@librechat/client';
import type { FC } from 'react';
import { EditPresetDialog, PresetItems } from './Presets';
import { useLocalize, usePresets } from '~/hooks';
import { useChatContext } from '~/Providers';

const PresetsMenu: FC = () => {
  const localize = useLocalize();
  const presetsMenuTriggerRef = useRef<HTMLDivElement>(null);
  const {
    presetsQuery,
    onSetDefaultPreset,
    onFileSelected,
    onSelectPreset,
    onChangePreset,
    clearAllPresets,
    onDeletePreset,
    submitPreset,
    exportPreset,
    showDeleteDialog,
    setShowDeleteDialog,
    presetToDelete,
    confirmDeletePreset,
  } = usePresets();
  const { preset } = useChatContext();
Repository: danny-avila/LibreChat
File: api/server/index.js (JavaScript)

Code before:
      messageCount++;
    }

    return;
  }

  if (err.message.includes('OpenAIError') || err.message.includes('ChatCompletionMessage')) {
    logger.error(
      '\n\nAn Uncaught `OpenAIError` error may be due to your reverse-proxy setup or stream configuration, or a bug in the `openai` node package.',
    );
    return;
  }

  if (err.stack && err.stack.includes('@librechat/agents')) {
    logger.error(
      '\n\nAn error occurred in the agents system. The error has been logged and the app will continue running.',
      {
        message: err.message,
        stack: err.stack,
      },
    );
    return;
  }

  logger.error('Unhandled error encountered. The app will continue running.', {
    name: err.name,
    message: err.message,
    stack: err.stack,
  });
});

/** Export app for easier testing purposes */
module.exports = app;


Review comment (suggestion):
some optional chaining and default in the case that `err` is nullish/undefined would be good here

Code after:
      messageCount++;
    }

    return;
  }

  if (err.message.includes('OpenAIError') || err.message.includes('ChatCompletionMessage')) {
    logger.error(
      '\n\nAn Uncaught `OpenAIError` error may be due to your reverse-proxy setup or stream configuration, or a bug in the `openai` node package.',
    );
    return;
  }

  if (err.stack && err.stack.includes('@librechat/agents')) {
    logger.error(
      '\n\nAn error occurred in the agents system. The error has been logged and the app will continue running.',
      {
        message: err.message,
        stack: err.stack,
      },
    );
    return;
  }

  if (isEnabled(process.env.CONTINUE_ON_UNCAUGHT_EXCEPTION)) {
    logger.error('Unhandled error encountered. The app will continue running.', {
      name: err?.name,
      message: err?.message,
      stack: err?.stack,
    });
    return;
  }

  process.exit(1);
});

/** Export app for easier testing purposes */
module.exports = app;
Repository: danny-avila/LibreChat
File: api/server/index.js (JavaScript)

Code before:
      logger.warn('Meilisearch error, search will be disabled');
      messageCount++;
    }

    return;
  }

  if (err.message.includes('OpenAIError') || err.message.includes('ChatCompletionMessage')) {
    logger.error(
      '\n\nAn Uncaught `OpenAIError` error may be due to your reverse-proxy setup or stream configuration, or a bug in the `openai` node package.',
    );
    return;
  }

  if (err.stack && err.stack.includes('@librechat/agents')) {
    logger.error(
      '\n\nAn error occurred in the agents system. The error has been logged and the app will continue running.',
      {
        message: err.message,
        stack: err.stack,
      },
    );
    return;
  }

  if (isEnabled(process.env.CRASH_ON_UNCAUGHT_EXCEPTION)) {
    process.exit(1);
  }

  logger.error('Unhandled error encountered. The app will continue running.', {
    name: err?.name,
    message: err?.message,
    stack: err?.stack,
  });
});

/** Export app for easier testing purposes */
module.exports = app;


Review comment (bug):
since crashing was the default behavior before, it should be the opposite logic, "CONTINUE_ON_UNCAUGHT_EXCEPTION" 

Code after:
      logger.warn('Meilisearch error, search will be disabled');
      messageCount++;
    }

    return;
  }

  if (err.message.includes('OpenAIError') || err.message.includes('ChatCompletionMessage')) {
    logger.error(
      '\n\nAn Uncaught `OpenAIError` error may be due to your reverse-proxy setup or stream configuration, or a bug in the `openai` node package.',
    );
    return;
  }

  if (err.stack && err.stack.includes('@librechat/agents')) {
    logger.error(
      '\n\nAn error occurred in the agents system. The error has been logged and the app will continue running.',
      {
        message: err.message,
        stack: err.stack,
      },
    );
    return;
  }

  if (isEnabled(process.env.CONTINUE_ON_UNCAUGHT_EXCEPTION)) {
    logger.error('Unhandled error encountered. The app will continue running.', {
      name: err?.name,
      message: err?.message,
      stack: err?.stack,
    });
    return;
  }

  process.exit(1);
});

/** Export app for easier testing purposes */
module.exports = app;
Repository: dask/dask
File: dask/dataframe/shuffle.py (Python)

Code before:
########################################################
# Various convenience functions to be run by the above #
########################################################


def partitioning_index(df, npartitions):
    """
    Computes a deterministic index mapping each record to a partition.

    Identical rows are mapped to the same partition.

    Parameters
    ----------
    df : DataFrame/Series/Index
    npartitions : int
        The number of partitions to group into.

    Returns
    -------
    partitions : ndarray
        An array of int64 values mapping each record to a partition.
    """
    if hasattr(df, "hash_columns"):
        return df.hash_columns() % int(npartitions)
    else:
        return hash_pandas_object(df, index=False) % int(npartitions)


def barrier(args):
    list(args)
    return 0


def collect(p, part, meta, barrier_token):
    """ Collect partitions from partd, yield dataframes """
    res = p.get(part)
    return res if len(res) > 0 else meta


def set_partitions_pre(s, divisions):
    partitions = divisions.searchsorted(s, side="right") - 1
    partitions[(s >= divisions.iloc[-1]).values] = len(divisions) - 2
    return partitions


def shuffle_group_2(df, col):
    if not len(df):
        return {}, df
    # `groupsort_indexer` cannot handle cupy.
    # Check if we need to get "host values" from cudf
    if hasattr(df, "values_host"):

Review comment (suggestion):
If there is no clear convention on how a dataframe-like object should produce a consistent hash of its rows (cc @TomAugspurger ?) then I recommend that we make a `Dispatch` function, as we've done with `concat`.  This would allow any other implementation to come in and act on equal footing to pandas or cudf.  

Code after:


def partitioning_index(df, npartitions):
    """
    Computes a deterministic index mapping each record to a partition.

    Identical rows are mapped to the same partition.

    Parameters
    ----------
    df : DataFrame/Series/Index
    npartitions : int
        The number of partitions to group into.

    Returns
    -------
    partitions : ndarray
        An array of int64 values mapping each record to a partition.
    """
    return methods.hash_df(df) % int(npartitions)


def barrier(args):
    list(args)
    return 0


def collect(p, part, meta, barrier_token):
    """ Collect partitions from partd, yield dataframes """
    res = p.get(part)
    return res if len(res) > 0 else meta


def set_partitions_pre(s, divisions):
    partitions = divisions.searchsorted(s, side="right") - 1
    partitions[(s >= divisions.iloc[-1]).values] = len(divisions) - 2
    return partitions


def shuffle_group_2(df, col):
    if not len(df):
        return {}, df
    ind = df[col].astype(np.int64)
    n = ind.max() + 1
    result2 = methods.group_split(df, ind.values.view(np.int64), n)
    return result2, df.iloc[:0]


def shuffle_group_get(g_head, i):
    g, head = g_head
    if i in g:
Repository: dask/dask
File: dask/dataframe/methods.py (Python)

Code before:
            if ind is None:
                ind = concat([df.index for df in dfs2])
            return pd.Series(union_categoricals(dfs2), index=ind, name=dfs2[0].name)
        with warnings.catch_warnings():
            if filter_warning:
                warnings.simplefilter("ignore", FutureWarning)
            out = pd.concat(dfs2, join=join, **concat_kwargs)
    # Re-add the index if needed
    if ind is not None:
        out.index = ind
    return out


# ---------------------------------
# hash_df
# ---------------------------------


hash_df_dispatch = Dispatch("hash_df")


def hash_df(dfs):
    """Hash each row of df, returning a Series-like object
    """
    func = hash_df_dispatch.dispatch(type(dfs))
    return func(dfs)


@hash_df_dispatch.register((pd.DataFrame, pd.Series, pd.Index))
def hash_df_pandas(dfs):
    return hash_pandas_object(dfs, index=False)


# ---------------------------------
# group_scatter
# ---------------------------------


group_scatter_dispatch = Dispatch("group_scatter")


def group_scatter(dfs, ind, stage, k, npartitions):
    """Scatter dataframe into "k" parts, using index "ind"
    """
    func = group_scatter_dispatch.dispatch(type(dfs))
    return func(dfs, ind, stage, k, npartitions)


@group_scatter_dispatch.register((pd.DataFrame, pd.Series, pd.Index))
def group_scatter_pandas(df, ind, stage, k, npartitions):
    c = ind.values

Review comment (refactor):
I think that this function may not be necessary, the `hash_df_dispatch` object above should do this.  You might remove this function and rename the dispatch object above.

We had to do tricky dispatch stuff with `concat` (where I expect you're copying things from) because `concat` takes in a list, and so it's harder to dispatch on types.

Code after:
            return pd.Series(union_categoricals(dfs2), index=ind, name=dfs2[0].name)
        with warnings.catch_warnings():
            if filter_warning:
                warnings.simplefilter("ignore", FutureWarning)
            out = pd.concat(dfs2, join=join, **concat_kwargs)
    # Re-add the index if needed
    if ind is not None:
        out.index = ind
    return out


def assign_index(df, ind):
    df = df.copy()
    df.index = ind
    return df


# ---------------------------------
# Shuffling/merging/sorting
# ---------------------------------


hash_df = Dispatch("hash_df")


@hash_df.register((pd.DataFrame, pd.Series, pd.Index))
def hash_df_pandas(dfs):
    return hash_pandas_object(dfs, index=False)


group_split = Dispatch("group_split")


@group_split.register((pd.DataFrame, pd.Series, pd.Index))
def group_split_pandas(df, c, k):
    indexer, locations = groupsort_indexer(c, k)
    df2 = df.take(indexer)
    locations = locations.cumsum()
    parts = [df2.iloc[a:b] for a, b in zip(locations[:-1], locations[1:])]
    return dict(zip(range(k), parts))
Repository: dask/dask
File: dask/sizeof.py (Python)

Code before:
@sizeof.register_lazy("scipy")
def register_spmatrix():
    from scipy import sparse

    @sizeof.register(sparse.dok_matrix)
    def sizeof_spmatrix_dok(s):
        return s.__sizeof__()

    @sizeof.register(sparse.spmatrix)
    def sizeof_spmatrix(s):
        return sum(sizeof(v) for v in s.__dict__.values())


@sizeof.register_lazy("pyarrow")
def register_pyarrow():
    import pyarrow as pa

    def _get_col_size(col):
        p = 0
        for chunk in col.data.iterchunks():
            for buffer in chunk.buffers():
                if buffer:
                    p += buffer.size
        return p

    @sizeof.register(pa.lib.Table)
    def sizeof_pyarrow_table(table):
        p = sizeof(table.schema.metadata)
        for col in table.itercolumns():
            p += _get_col_size(col)
        return int(p) + 1000

    @sizeof.register(pa.lib.Column)
    def sizeof_pyarrow_column(col):
        return int(_get_col_size(col)) + 1000


Review comment (suggestion):
Is pyarrow.lib public? Or should this use `pa.Table`?

same question for `pa.lib.Column` below.

Code after:

@sizeof.register_lazy("scipy")
def register_spmatrix():
    from scipy import sparse

    @sizeof.register(sparse.dok_matrix)
    def sizeof_spmatrix_dok(s):
        return s.__sizeof__()

    @sizeof.register(sparse.spmatrix)
    def sizeof_spmatrix(s):
        return sum(sizeof(v) for v in s.__dict__.values())


@sizeof.register_lazy("pyarrow")
def register_pyarrow():
    import pyarrow as pa

    def _get_col_size(data):
        p = 0
        if not isinstance(data, pa.ChunkedArray):
            data = data.data  # pyarrow <0.15.0
        for chunk in data.iterchunks():
            for buffer in chunk.buffers():
                if buffer:
                    p += buffer.size
        return p

    @sizeof.register(pa.Table)
    def sizeof_pyarrow_table(table):
        p = sizeof(table.schema.metadata)
        for col in table.itercolumns():
            p += _get_col_size(col)
        return int(p) + 1000

    @sizeof.register(pa.ChunkedArray)
    def sizeof_pyarrow_chunked_array(data):
        return int(_get_col_size(data)) + 1000

    # Handle pa.Column for pyarrow < 0.15
    if pa.__version__ < LooseVersion("0.15.0"):

        @sizeof.register(pa.Column)
        def sizeof_pyarrow_column(col):
            return int(_get_col_size(col)) + 1000
Repository: dask/dask
File: dask/dataframe/io/parquet/core.py (Python)

Code before:
    if index_in_columns:
        meta = meta.set_index(index)

    if len(divisions) < 2:
        # empty dataframe - just use meta
        subgraph = {(name, 0): meta}
        divisions = (None, None)

    return new_dd_object(subgraph, name, meta, divisions)


def read_parquet_part(func, fs, meta, part, columns, index, kwargs):
    """ Read a part of a parquet dataset

    This function is used by `read_parquet`."""
    if isinstance(part, list):
        concat_func = kwargs.pop("concat_func", False)
        if not concat_func:
            import pandas as pd

            concat_func = pd.concat

        dfs = []
        for rg in part:
            df = func(fs, rg, columns.copy(), index, **kwargs)
            dfs.append(df)
        df = concat_func(dfs, axis=0)
    else:
        df = func(fs, part, columns, index, **kwargs)

    if meta.columns.name:
        df.columns.name = meta.columns.name
    columns = columns or []
    index = index or []
    return df[[c for c in columns if c not in index]]


def to_parquet(
    df,
    path,
    engine="auto",
    compression="default",
    write_index=True,
    append=False,
    ignore_divisions=False,
    partition_on=None,
    storage_options=None,
    write_metadata_file=True,
    compute=True,
    **kwargs
):

Review comment (suggestion):
Can I ask for a list comprension here rather than a for loop?  

```python
if isinstance(part, list):
    dfs = [func(fs, rg, columns.copy(), index, **kwargs) for rf in part]
    df = methods.concat(dfs, axiis=0)
```

Code after:
    # Set the index that was previously treated as a column
    if index_in_columns:
        meta = meta.set_index(index)

    if len(divisions) < 2:
        # empty dataframe - just use meta
        subgraph = {(name, 0): meta}
        divisions = (None, None)

    return new_dd_object(subgraph, name, meta, divisions)


def read_parquet_part(func, fs, meta, part, columns, index, kwargs):
    """ Read a part of a parquet dataset

    This function is used by `read_parquet`."""
    if isinstance(part, list):
        dfs = [func(fs, rg, columns.copy(), index, **kwargs) for rg in part]
        df = concat(dfs, axis=0)
    else:
        df = func(fs, part, columns, index, **kwargs)

    if meta.columns.name:
        df.columns.name = meta.columns.name
    columns = columns or []
    index = index or []
    return df[[c for c in columns if c not in index]]


def to_parquet(
    df,
    path,
    engine="auto",
    compression="default",
    write_index=True,
    append=False,
    ignore_divisions=False,
    partition_on=None,
    storage_options=None,
    write_metadata_file=True,
    compute=True,
    **kwargs
):
    """Store Dask.dataframe to Parquet files

    Notes
    -----
    Each partition will be written to a separate file.

    Parameters
    ----------
Repository: dask/dask
File: dask/dataframe/io/tests/test_parquet.py (Python)

Code before:
        row_group_size=row_group_size,
        write_metadata_file=metadata,
    )

    if metadata:
        path = str(tmpdir)
    else:
        dirname = str(tmpdir)
        files = os.listdir(dirname)
        assert "_metadata" not in files
        path = os.path.join(dirname, "*.parquet")

    ddf2 = dd.read_parquet(
        path,
        engine=engine,
        chunksize=chunksize,
        split_row_groups=True,
        gather_statistics=True,
        index="index",
    )

    assert_eq(ddf1, ddf2, check_divisions=False)

    for df_part in ddf2.partitions:
        if chunksize and len(df_part) > row_group_size:
            assert df_part.compute().memory_usage().sum() <= parse_bytes(chunksize)


Review comment (suggestion):
I don't think that this test checks that we actually merge things.  Maybe something like the following would work?

```python
def test_chunksize(tmpdir, engine):
    df = # make a dataframe with 10 small partitions
    df.to_parquet(...)

    df2 = dd.read_parquet(..., chunksize="2 kB")  # or something like this.
    assert df2.npartitions == 2 # or whatever the expected value is
```

Code after:
    ddf1.to_parquet(
        str(tmpdir),
        engine="pyarrow",
        row_group_size=row_group_size,
        write_metadata_file=metadata,
    )

    if metadata:
        path = str(tmpdir)
    else:
        dirname = str(tmpdir)
        files = os.listdir(dirname)
        assert "_metadata" not in files
        path = os.path.join(dirname, "*.parquet")

    ddf2 = dd.read_parquet(
        path,
        engine=engine,
        chunksize=chunksize,
        split_row_groups=True,
        gather_statistics=True,
        index="index",
    )

    assert_eq(ddf1, ddf2, check_divisions=False)

    num_row_groups = df_size // row_group_size
    if not chunksize:
        assert ddf2.npartitions == num_row_groups
    else:
        # Check that we are really aggregating
        df_byte_size = row_group_byte_size * num_row_groups
        expected = df_byte_size // parse_bytes(chunksize)
        remainder = (df_byte_size % parse_bytes(chunksize)) > 0
        expected += int(remainder) * nparts
        assert ddf2.npartitions == max(nparts, expected)
Repository: dask/dask
File: dask/dataframe/shuffle.py (Python)

Code before:
    --------
    set_index
    set_partition
    shuffle_disk
    """
    if not isinstance(index, _Frame):
        index = df._select_columns_or_index(index)

    partitions = index.map_partitions(
        partitioning_index,
        npartitions=npartitions or df.npartitions,
        meta=df._meta._constructor_sliced([0]),
        transform_divisions=False,
    )
    df2 = df.assign(_partitions=partitions)
    df2._meta.index.name = df._meta.index.name
    df3 = rearrange_by_column(
        df2,
        "_partitions",
        npartitions=npartitions,
        max_branch=max_branch,
        shuffle=shuffle,
        compute=compute,
    )
    return df3.drop(columns=["_partitions"])


def rearrange_by_divisions(df, column, divisions, max_branch=None, shuffle=None):
    """ Shuffle dataframe so that column separates along divisions """
    divisions = df._meta._constructor_sliced(divisions)
    meta = df._meta._constructor_sliced([0])
    # Assign target output partitions to every row
    partitions = df[column].map_partitions(
        set_partitions_pre, divisions=divisions, meta=meta
    )
    df2 = df.assign(_partitions=partitions)

    # Perform shuffle
    df3 = rearrange_by_column(
        df2,
        "_partitions",
        max_branch=max_branch,
        npartitions=len(divisions) - 1,
        shuffle=shuffle,
    )
    return df3.drop(columns=["_partitions"])


def rearrange_by_column(
    df, col, npartitions=None, max_branch=None, shuffle=None, compute=None
):

Review comment (suggestion):
I would hope that our use of `del` more generally would also benefit from this.  So hopefully this modification wouldn't be necessary.

Code after:
    --------
    set_index
    set_partition
    shuffle_disk
    """
    if not isinstance(index, _Frame):
        index = df._select_columns_or_index(index)

    partitions = index.map_partitions(
        partitioning_index,
        npartitions=npartitions or df.npartitions,
        meta=df._meta._constructor_sliced([0]),
        transform_divisions=False,
    )
    df2 = df.assign(_partitions=partitions)
    df2._meta.index.name = df._meta.index.name
    df3 = rearrange_by_column(
        df2,
        "_partitions",
        npartitions=npartitions,
        max_branch=max_branch,
        shuffle=shuffle,
        compute=compute,
    )
    del df3["_partitions"]
    return df3


def rearrange_by_divisions(df, column, divisions, max_branch=None, shuffle=None):
    """ Shuffle dataframe so that column separates along divisions """
    divisions = df._meta._constructor_sliced(divisions)
    meta = df._meta._constructor_sliced([0])
    # Assign target output partitions to every row
    partitions = df[column].map_partitions(
        set_partitions_pre, divisions=divisions, meta=meta
    )
    df2 = df.assign(_partitions=partitions)

    # Perform shuffle
    df3 = rearrange_by_column(
        df2,
        "_partitions",
        max_branch=max_branch,
        npartitions=len(divisions) - 1,
        shuffle=shuffle,
    )
    del df3["_partitions"]
    return df3


def rearrange_by_column(