ronantakizawa/codereview-bench huggingface dataset:
Repository: StanGirard/quivr
File: frontend/app/user/components/LanguageSelect/hooks/useLanguageHook.ts (TypeScript)

Code before:
  },
  {
    label: "简体中文",
    flag: "🇨🇳",
    shortName: "zh",
  },
];

export const useLanguageHook = (): {
  change: (newLanguage: Language) => void;
  allLanguages: Language[];
  currentLanguage: Language | undefined;
} => {
  const { i18n } = useTranslation();
  const [allLanguages, setAllLanguages] = useState<Language[]>([]);
  const [currentLanguage, setCurrentLanguage] = useState<Language>();
  const { track } = useEventTracking();

  useEffect(() => {
    setAllLanguages(languages);
    const savedLanguage = localStorage.getItem("selectedLanguage") ?? "English";

    setCurrentLanguage(
        languages.find((language) => language.label === savedLanguage)
    );
    var shortName = languages.find(
        (language) => language.label === savedLanguage
    )?.shortName;
    void i18n.changeLanguage(shortName);
}, [i18n]);

  const change = (newLanguage: Language) => {
    void track("CHANGE_LANGUAGE");
    setCurrentLanguage(newLanguage);
    localStorage.setItem("selectedLanguage", newLanguage.label);
    void i18n.changeLanguage(newLanguage.shortName);
  };

  return {
    change,
    allLanguages,
    currentLanguage,
  };
};


Review comment (suggestion):
```suggestion
    const shortName = languages.find(
```

Code after:
  },
  {
    label: "简体中文",
    flag: "🇨🇳",
    shortName: "zh",
  },
];

export const useLanguageHook = (): {
  change: (newLanguage: Language) => void;
  allLanguages: Language[];
  currentLanguage: Language | undefined;
} => {
  const { i18n } = useTranslation();
  const [allLanguages, setAllLanguages] = useState<Language[]>([]);
  const [currentLanguage, setCurrentLanguage] = useState<Language>();
  const { track } = useEventTracking();

  useEffect(() => {
    setAllLanguages(languages);
    const savedLanguage = localStorage.getItem("selectedLanguage") ?? "English";
    let choosedLanguage = languages.find(
      (lang) => lang.label === savedLanguage
    );
    if (!choosedLanguage) {
      choosedLanguage = languages.find((lang) => lang.label === "English");
    }
    if (currentLanguage) {
      setCurrentLanguage(choosedLanguage);
      localStorage.setItem("selectedLanguage", currentLanguage.label);
      void i18n.changeLanguage(currentLanguage.shortName);
    } else {
      console.error(
        "No valid language found, please check the languages configuration."
      );
    }
  }, [i18n]);

  const change = (newLanguage: Language) => {
    void track("CHANGE_LANGUAGE");
    setCurrentLanguage(newLanguage);
    localStorage.setItem("selectedLanguage", newLanguage.label);
    void i18n.changeLanguage(newLanguage.shortName);
  };

  return {
    change,
    allLanguages,
    currentLanguage,
  };
};
Repository: StanGirard/quivr
File: backend/modules/user/controller/user_controller.py (Python)

Code before:
)
def get_user_identity_route(
    current_user: UserIdentity = Depends(get_current_user),
) -> UserIdentity:
    """
    Get user identity.
    """
    return user_repository.get_user_identity(current_user.id)

@user_router.delete(
    "/user/{user_id}",
    dependencies=[Depends(AuthBearer())],
    tags=["User"],
)
async def delete_user_route(
    user_id: str
):
    """
    Delete a user.

    - `user_id`: The ID of the user to delete.

    This endpoint deletes a user from the system.
    """

    user_repository.delete_user(user_id)

    return {"message": "User deleted successfully"}

@user_router.get(
    "/user/credits",
    dependencies=[Depends(AuthBearer())],
    tags=["User"],
)
def get_user_credits(
    current_user: UserIdentity = Depends(get_current_user),
) -> int:
    """
    Get user remaining credits.
    """
    return user_repository.get_user_credits(current_user.id)


Review comment (suggestion):
tu ne vérifies pas si c'est bien l'utilisateur. Regarde comment c'est fait avec les current_user qui récupère l'id du user depuis le token JWT. 

Ici dans cette example un utilisateur mal intentionné peut supprimer n'importe qui 

Code after:
)
def get_user_identity_route(
    current_user: UserIdentity = Depends(get_current_user),
) -> UserIdentity:
    """
    Get user identity.
    """
    return user_repository.get_user_identity(current_user.id)

@user_router.delete(
    "/user_data",
    dependencies=[Depends(AuthBearer())],
    tags=["User"],
)
async def delete_user_data_route(
    current_user: UserIdentity = Depends(get_current_user),
):
    """
    Delete a user.

    - `user_id`: The ID of the user to delete.

    This endpoint deletes a user from the system.
    """

    user_repository.delete_user_data(current_user.id)

    return {"message": "User deleted successfully"}

@user_router.get(
    "/user/credits",
    dependencies=[Depends(AuthBearer())],
    tags=["User"],
)
def get_user_credits(
    current_user: UserIdentity = Depends(get_current_user),
) -> int:
    """
    Get user remaining credits.
    """
    return user_repository.get_user_credits(current_user.id)
Repository: StanGirard/quivr
File: frontend/lib/components/TextEditor/TextEditor.tsx (TypeScript)

Code before:
          openOnClick: false,
        }),
        BrainMention,
        FocusSearchBar,
        AIHighlight.configure({
          HTMLAttributes: {
            class: styles.ai_highlight,
          },
        }),
      ],
      content: defaultContent,
      immediatelyRender: false,
      autofocus: true,
    },
    [items.length]
  );

  const toggleSearchBar = () => {
    if (searchBarOpen) {
      setSearchBarOpen(false);
      editor?.commands.focus();
    } else {
      setSearchBarOpen(true);
      searchEditorRef.current?.commands.focus();
    }
  };

  if (!editor) {
    return <></>;
  }

  return (
    <div className={styles.main_container}>
      <div className={styles.editor_wrapper}>
        <Toolbar toggleSearchBar={toggleSearchBar} editor={editor} />
        <div>
          <BubbleMenu
            shouldShow={() => {
              return editor.isActive("aiHighlight", { type: "ai" });
            }}
            tippyOptions={{
              moveTransition: "transform 0.1s",
              placement: "bottom-start",
            }}
            className={styles.bubble_menu}
            editor={editor}
          >
            <QuivrButton
              onClick={() => {
                editor
                  .chain()

Review comment (suggestion):
```suggestion
const toggleSearchBar = () => {
  setSearchBarOpen(!searchBarOpen);
  (searchBarOpen ? editor : searchEditorRef.current)?.commands.focus();
};
```

Code after:
          openOnClick: false,
        }),
        BrainMention,
        FocusSearchBar,
        AIHighlight.configure({
          HTMLAttributes: {
            class: styles.ai_highlight,
          },
        }),
      ],
      content: defaultContent,
      immediatelyRender: false,
      autofocus: true,
    },
    [items.length]
  );

  const toggleSearchBar = () => {
    setSearchBarOpen(!searchBarOpen);
    (searchBarOpen ? editor : searchEditorRef.current)?.commands.focus();
  };

  if (!editor) {
    return <></>;
  }

  return (
    <div className={styles.main_container}>
      <div className={styles.editor_wrapper}>
        <Toolbar toggleSearchBar={toggleSearchBar} editor={editor} />
        <div>
          <BubbleMenu
            shouldShow={() => {
              return editor.isActive(AIHighlight.name, { type: "ai" });
            }}
            tippyOptions={{
              moveTransition: "transform 0.1s",
              placement: "bottom-start",
            }}
            className={styles.bubble_menu}
            editor={editor}
          >
            <QuivrButton
              onClick={() => {
                editor
                  .chain()
                  .extendMarkRange(AIHighlight.name)
                  .unsetHighlight()
                  .run();
              }}
              label="Accept"
Repository: Stirling-Tools/Stirling-PDF
File: frontend/src/types/automation.ts (TypeScript)

Code before:
 * Types for automation functionality
 */

export interface AutomationOperation {
  operation: string;
  parameters: Record<string, any>;
}

export interface AutomationConfig {
  id: string;
  name: string;
  description?: string;
  operations: AutomationOperation[];
  createdAt: string;
  updatedAt: string;
}

export interface AutomationTool {
  id: string;
  operation: string;
  name: string;
  configured: boolean;
  parameters?: Record<string, any>;
}

export interface AutomationStepData {
  step: 'selection' | 'creation' | 'run';
  mode?: AutomationMode;
  automation?: AutomationConfig;
}

export interface ExecutionStep {
  id: string;
  operation: string;
  name: string;
  status: 'pending' | 'running' | 'completed' | 'error';
  error?: string;
}

export interface AutomationExecutionCallbacks {
  onStepStart?: (stepIndex: number, operationName: string) => void;
  onStepComplete?: (stepIndex: number, resultFiles: File[]) => void;
  onStepError?: (stepIndex: number, error: string) => void;
}

export interface AutomateParameters extends AutomationExecutionCallbacks {
  automationConfig?: AutomationConfig;
}

export enum AutomationMode {
  CREATE = 'create',

Review comment (suggestion):
Is there some benefit to doing this instead of just using a standard import? This is really hard to read compared to just importing `AUTOMATION_STEPS` 

Code after:
 * Types for automation functionality
 */

export interface AutomationOperation {
  operation: string;
  parameters: Record<string, any>;
}

export interface AutomationConfig {
  id: string;
  name: string;
  description?: string;
  operations: AutomationOperation[];
  createdAt: string;
  updatedAt: string;
}

export interface AutomationTool {
  id: string;
  operation: string;
  name: string;
  configured: boolean;
  parameters?: Record<string, any>;
}

export type AutomationStep = typeof import('../constants/automation').AUTOMATION_STEPS[keyof typeof import('../constants/automation').AUTOMATION_STEPS];

export interface AutomationStepData {
  step: AutomationStep;
  mode?: AutomationMode;
  automation?: AutomationConfig;
}

export interface ExecutionStep {
  id: string;
  operation: string;
  name: string;
  status: 'pending' | 'running' | 'completed' | 'error';
  error?: string;
}

export interface AutomationExecutionCallbacks {
  onStepStart?: (stepIndex: number, operationName: string) => void;
  onStepComplete?: (stepIndex: number, resultFiles: File[]) => void;
  onStepError?: (stepIndex: number, error: string) => void;
}

export interface AutomateParameters extends AutomationExecutionCallbacks {
  automationConfig?: AutomationConfig;
}
Repository: Stirling-Tools/Stirling-PDF
File: app/common/src/main/java/stirling/software/common/util/PDFService.java (Java)

Code before:

import org.apache.pdfbox.pdmodel.PDDocument;
import org.apache.pdfbox.pdmodel.PDPage;
import org.springframework.stereotype.Service;

import lombok.RequiredArgsConstructor;

import stirling.software.common.service.CustomPDFDocumentFactory;

@Service
@RequiredArgsConstructor
public class PDFService {

    private final CustomPDFDocumentFactory pdfDocumentFactory;

    /**
     * Merges the given documents into a new PDDocument. Caller owns/should close the result.
     *
     * @param documents The list of PDDocuments to merge
     * @return A new PDDocument containing the merged pages
     * @throws IOException If an error occurs during merging
     */
    public PDDocument mergeDocuments(List<PDDocument> documents) throws IOException {
        PDDocument merged = pdfDocumentFactory.createNewDocument();
        for (PDDocument doc : documents) {
            for (PDPage page : doc.getPages()) {
                merged.addPage(page);
            }
        }
        return merged;
    }
}


Review comment (suggestion):
I think directly adding pages from other docs might cause issues, since those PDPage objects belong to their original document. Should we be importing/cloning the pages instead? or using the PDFBox merge util

Code after:

import org.apache.pdfbox.multipdf.PDFMergerUtility;
import org.apache.pdfbox.pdmodel.PDDocument;
import org.springframework.stereotype.Service;

import lombok.RequiredArgsConstructor;

import stirling.software.common.service.CustomPDFDocumentFactory;

@Service
@RequiredArgsConstructor
public class PDFService {

    private final CustomPDFDocumentFactory pdfDocumentFactory;

    /*
     * Merge multiple PDF documents into a single PDF document
     *
     * @param documents List of PDDocument to be merged
     * @return Merged PDDocument
     * @throws IOException If an error occurs during merging
     */
    public PDDocument mergeDocuments(List<PDDocument> documents) throws IOException {
        PDDocument merged = pdfDocumentFactory.createNewDocument();
        PDFMergerUtility merger = new PDFMergerUtility();
        for (PDDocument doc : documents) {
            merger.appendDocument(merged, doc);
        }
        return merged;
    }
}
Repository: Stirling-Tools/Stirling-PDF
File: app/core/src/main/java/stirling/software/SPDF/model/api/misc/FlattenRequest.java (Java)

Code before:
package stirling.software.SPDF.model.api.misc;

import io.swagger.v3.oas.annotations.media.Schema;

import lombok.Data;
import lombok.EqualsAndHashCode;

import stirling.software.common.model.api.PDFFile;

@Data
@EqualsAndHashCode(callSuper = true)
public class FlattenRequest extends PDFFile {

    @Schema(
            description =
                    "True to flatten only the forms, false to flatten full PDF (Convert page to"
                            + " image)",
            requiredMode = Schema.RequiredMode.REQUIRED,
            defaultValue = "false")
    private Boolean flattenOnlyForms;

    @Schema(
            description = "Optional DPI for page rendering when flattening the full document.",
            requiredMode = Schema.RequiredMode.NOT_REQUIRED,
            example = "100",
            minimum = "72")
    private Integer renderDpi;
}


Review comment (suggestion):
I would omit `example`, as it's optional. It could be confusing, so perhaps it should be added to the `description`.

Code after:
package stirling.software.SPDF.model.api.misc;

import io.swagger.v3.oas.annotations.media.Schema;

import lombok.Data;
import lombok.EqualsAndHashCode;

import stirling.software.common.model.api.PDFFile;

@Data
@EqualsAndHashCode(callSuper = true)
public class FlattenRequest extends PDFFile {

    @Schema(
            description =
                    "True to flatten only the forms, false to flatten full PDF (Convert page to"
                            + " image)",
            requiredMode = Schema.RequiredMode.REQUIRED,
            defaultValue = "false")
    private Boolean flattenOnlyForms;

    @Schema(
            description = "Optional DPI for page rendering when flattening the full document.",
            requiredMode = Schema.RequiredMode.NOT_REQUIRED,
            minimum = "72")
    private Integer renderDpi;
}
Repository: SuperDuperDB/superduperdb
File: superduperdb/cdc/app.py (Python)

Code before:
from superduperdb import CFG
from superduperdb.base.datalayer import Datalayer
from superduperdb.components.listener import Listener
from superduperdb.server import app as superduperapp

assert isinstance(CFG.cluster.cdc, str), "Missing SUPERDUPERDB_CLUSTER_CDC"
port = int(CFG.cluster.cdc.split(':')[-1])
app = superduperapp.SuperDuperApp('cdc', port=port)


@app.startup
def cdc_startup(db: Datalayer):
    db.cdc.start()


@app.add('/listener/add', method='get')
def add_listener(name: str, db: Datalayer = superduperapp.DatalayerDependency()):
    '''
    Endpoint for adding a listener to cdc
    '''
    listener = db.load('listener', name)
    assert isinstance(listener, Listener)
    db.cdc.add(listener)


@app.add('/listener/delete', method='get')
def remove_listener(name: str, db: Datalayer = superduperapp.DatalayerDependency()):
    '''
    Endpoint for removing a listener from cdc
    '''
    listener = db.load('listener', name)

Review comment (suggestion):
`SUPERDUPERDB_CLUSTER_CDC` is a way to set, CFG.cluster.cdc 

so assertion error should be

`cluster.cdc should be set with a valid uri`
something on the above terms.

Code after:
from superduperdb import CFG
from superduperdb.base.datalayer import Datalayer
from superduperdb.components.listener import Listener
from superduperdb.server import app as superduperapp

assert isinstance(CFG.cluster.cdc, str), "cluster.cdc should be set with a valid uri"
port = int(CFG.cluster.cdc.split(':')[-1])
app = superduperapp.SuperDuperApp('cdc', port=port)


@app.startup
def cdc_startup(db: Datalayer):
    db.cdc.start()


@app.add('/listener/add', method='get')
def add_listener(name: str, db: Datalayer = superduperapp.DatalayerDependency()):
    """
    Endpoint for adding a listener to cdc
    """
    listener = db.load('listener', name)
    assert isinstance(listener, Listener)
    db.cdc.add(listener)


@app.add('/listener/delete', method='get')
def remove_listener(name: str, db: Datalayer = superduperapp.DatalayerDependency()):
    """
    Endpoint for removing a listener from cdc
    """
    listener = db.load('listener', name)
Repository: SuperDuperDB/superduperdb
File: superduperdb/base/exceptions.py (Python)

Code before:
from superduperdb import logging


class ComponentInUseError(Exception):
    pass


class ComponentInUseWarning(Warning):
    pass


class BaseException(Exception):
    '''
    BaseException which logs a message after
    exception
    '''

    def __init__(self, msg):
        self.msg = msg
        logging.exception(self.msg, e=self)

    def __str__(self):
        return self.msg


class ServiceRequestException(BaseException):
    '''
    ServiceRequestException
    '''


class QueryException(BaseException):
    '''
    QueryException
    '''


class DatabackendException(BaseException):
    '''
    DatabackendException
    '''


class MetadataException(BaseException):
    '''
    MetadataException
    '''


class ComponentException(BaseException):
    '''

Review comment (suggestion):
```python
class RequiredPackageNotFound(ImportError):
```

Code after:
from superduperdb import logging


class ComponentInUseError(Exception):
    pass


class ComponentInUseWarning(Warning):
    pass


class BaseException(Exception):
    '''
    BaseException which logs a message after
    exception
    '''

    def __init__(self, msg):
        self.msg = msg
        logging.exception(self.msg, e=self)

    def __str__(self):
        return self.msg


class RequiredPackageNotFound(ImportError):

    '''
    Exception raised when one or more required packages are not found.
    '''


class ServiceRequestException(BaseException):
    '''
    ServiceRequestException
    '''


class QueryException(BaseException):
    '''
    QueryException
    '''


class DatabackendException(BaseException):
    '''
    DatabackendException
    '''


class MetadataException(BaseException):
Repository: SuperDuperDB/superduperdb
File: test/rest/test_rest.py (Python)

Code before:


@pytest.fixture
def setup():
    yield _setup()
    teardown()


def test_select_data(setup):
    result = curl_post('/db/execute', data={'query': 'coll.find({}, {"_id": 0})'})
    print(result)
    assert len(result) == 2


CODE = """
from superduperdb import code

@code
def my_function(x):
    return x + 1
"""


def test_apply(setup):
    m = {
        '_leaves': {
            'function_body': {
                '_path': 'superduperdb/base/code/Code',
                'code': CODE,
            },
            'my_function': {
                '_path': 'superduperdb/components/model/ObjectModel',
                'object': '?function_body',
                'identifier': 'my_function',
            },
        },
        '_base': '?my_function',
    }

    _ = curl_post(
        endpoint='/db/apply',
        data=m,
    )

    models = curl_get('/db/show', params={'type_id': 'model'})

    assert models == ['my_function']


def test_insert_image(setup):
    result = curl_put(

Review comment (suggestion):
When handwriting dictionary data, retaining the _builds string instead of using KEY_BUILDS helps with readability.


The same logic applies to retaining the _builds field in the following test cases.

Code after:


@pytest.fixture
def setup():
    yield _setup()
    teardown()


def test_select_data(setup):
    result = curl_post('/db/execute', data={'query': 'coll.find({}, {"_id": 0})'})
    print(result)
    assert len(result) == 2


CODE = """
from superduperdb import code

@code
def my_function(x):
    return x + 1
"""


def test_apply(setup):
    m = {
        '_builds': {
            'function_body': {
                '_path': 'superduperdb/base/code/Code',
                'code': CODE,
            },
            'my_function': {
                '_path': 'superduperdb/components/model/ObjectModel',
                'object': '?function_body',
                'identifier': 'my_function',
            },
        },
        '_base': '?my_function',
    }

    _ = curl_post(
        endpoint='/db/apply',
        data=m,
    )

    models = curl_get('/db/show', params={'type_id': 'model'})

    assert models == ['my_function']


def test_insert_image(setup):
    result = curl_put(
Repository: SuperDuperDB/superduperdb
File: superduper/misc/auto_schema.py (Python)

Code before:
            for k, v in sorted(schema_data.items()):
                if hasattr(v, "identifier"):
                    key_value_pairs.append(f"{k}={v.identifier}")
                else:
                    key_value_pairs.append(f"{k}={str(v)}")
            identifier = "&".join(key_value_pairs)

    identifier = "AUTO-" + identifier

    return Schema(identifier=identifier, fields=schema_data)  # type: ignore


class JsonDataTypeFactory(DataTypeFactory):
    """A factory for JSON datatypes # noqa."""

    @staticmethod
    def check(data: t.Any) -> bool:
        """Check if the data is able to be encoded by the JSON serializer.

        :param data: The data object
        """
        try:
            json_serializer.encode_data(data)
            return True
        except Exception:
            return False

    @staticmethod
    def create(data: t.Any) -> DataType | FieldType:
        """Create a JSON datatype.

        :param data: The data object
        """
        if CFG.json_native:
            return FieldType(identifier='json')
        return json_serializer


Review comment (suggestion):
This doesn’t seem to support a list of floats. So for the vector data type, it must be numpy, right? I think it make sense.

Code after:
                    key_value_pairs.append(f"{k}={str(v)}")
            identifier = "&".join(key_value_pairs)

    identifier = "AUTO-" + identifier

    return Schema(identifier=identifier, fields=schema_data)  # type: ignore


class VectorTypeFactory(DataTypeFactory):
    """A factory for Vector datatypes # noqa."""

    @staticmethod
    def check(data: t.Any) -> bool:
        """Check if the data is able to be encoded by the JSON serializer.

        :param data: The data object
        """
        return isinstance(data, np.ndarray) and len(data.shape) == 1

    @staticmethod
    def create(data: t.Any) -> BaseDataType | FieldType:
        """Create a JSON datatype.

        :param data: The data object
        """
        return Vector(shape=(len(data),), dtype=str(data.dtype))


class JsonDataTypeFactory(DataTypeFactory):
    """A factory for JSON datatypes # noqa."""

    @staticmethod
    def check(data: t.Any) -> bool:
        """Check if the data is able to be encoded by the JSON serializer.

        :param data: The data object
        """
        try:
            json_serializer.encode_data(data)
            return True
        except Exception:
            return False

    @staticmethod
    def create(data: t.Any) -> BaseDataType | FieldType:
        """Create a JSON datatype.

        :param data: The data object
        """
        if CFG.json_native:
            return FieldType(identifier='json')