ronantakizawa/codereview-bench huggingface dataset:
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/redux/reducers/queries.ts (TypeScript)

Code before:
      if (!node) {
        return state
      }
      const queriesByNode = state.byNode.get(node.id) ?? []
      const queriesByConnection =
        state.byConnection.get(node.internal.type) ?? []

      queriesByNode.forEach(queryId => {
        const query = state.trackedQueries.get(queryId)
        if (query) {
          query.dirty = setFlag(query.dirty, FLAG_DIRTY_DATA)
        }
      })
      queriesByConnection.forEach(queryId => {
        const query = state.trackedQueries.get(queryId)
        if (query) {
          query.dirty = setFlag(query.dirty, FLAG_DIRTY_DATA)
        }
      })
      return state
    }
    case `PAGE_QUERY_RUN`: {
      const { path } = action.payload
      const query = registerQuery(state, path)
      query.dirty = 0
      query.running = 0 // TODO: also reset the state on pre-bootstrap (as it could've been persisted)
      return state
    }
    default:
      return state
  }
}

function setFlag(allFlags: number, flag: number, set = true): number {
  if (allFlags < 0) {
    allFlags = 0
  }
  return set ? allFlags | flag : allFlags & ~flag
}

export function hasFlag(allFlags: number, flag: number): boolean {
  return allFlags >= 0 && (allFlags & flag) > 0
}

function registerQuery(
  state: IGatsbyState["queries"],
  queryId: QueryId
): IQueryState {
  let query = state.trackedQueries.get(queryId)
  if (!query) {
    query = initialQueryState()

Review comment (suggestion):
should this TODO be implemented in this PR? I would think so, because again we could (maybe?) end up waiting for "running" query that doesn't actually run in current session

Code after:
      }
      for (const queryId of queriesByConnection) {
        const query = state.trackedQueries.get(queryId)
        if (query) {
          query.dirty = setFlag(query.dirty, FLAG_DIRTY_DATA)
        }
      }
      return state
    }
    case `PAGE_QUERY_RUN`: {
      const { path } = action.payload
      const query = registerQuery(state, path)
      query.dirty = 0
      query.running = 0 // TODO: also
      return state
    }
    case `SET_PROGRAM_STATUS`: {
      if (action.payload === `BOOTSTRAP_FINISHED`) {
        // Reset the running state (as it could've been persisted)
        for (const [, query] of state.trackedQueries) {
          query.running = 0
        }
      }
      return state
    }
    default:
      return state
  }
}

function setFlag(allFlags: number, flag: number, set = true): number {
  if (allFlags < 0) {
    allFlags = 0
  }
  return set ? allFlags | flag : allFlags & ~flag
}

export function hasFlag(allFlags: number, flag: number): boolean {
  return allFlags >= 0 && (allFlags & flag) > 0
}

function addNodeDependency(
  state: IGatsbyState["queries"],
  queryId: QueryId,
  nodeId: NodeId
): IGatsbyState["queries"] {
  // Perf: using two-side maps.
  //   Without additional `queryNodes` map we would have to loop through
  //   all existing nodes in `clearNodeDependencies` to delete node <-> query dependency
  let nodeQueries = state.byNode.get(nodeId)
  if (!nodeQueries) {
Repository: gatsbyjs/gatsby
File: packages/gatsby/src/utils/get-page-data.ts (TypeScript)

Code before:
import {
  IPageDataWithQueryResult,
  readPageData as readPageDataUtil,
} from "./page-data"

export interface IPageQueryResult {
  id: string
  result: IPageDataWithQueryResult
}

export async function getPageData(pagePath: string): Promise<IPageQueryResult> {
  const { queries } = store.getState()

  const query = queries.trackedQueries.get(pagePath)

  if (!query) {
    throw new Error(`Could not find query ${pagePath}`)
  }
  if (query.running !== 0) {
    return waitNextPageData(pagePath)
  }
  if (query.dirty !== 0) {
    store.dispatch(actions.queryRunRequested({ path: pagePath, isPage: true }))
    return waitNextPageData(pagePath)
  }
  // Results are up-to-date
  return readPageData(pagePath)
}

async function waitNextPageData(pagePath: string): Promise<IPageQueryResult> {
  return new Promise(resolve => {
    const listener = (data: IClearPendingPageDataWriteAction): void => {
      if (data.payload.page === pagePath) {
        emitter.off(`CLEAR_PENDING_PAGE_DATA_WRITE`, listener)
        resolve(readPageData(pagePath))
      }
    }
    emitter.on(`CLEAR_PENDING_PAGE_DATA_WRITE`, listener)
  })
}

async function readPageData(pagePath): Promise<IPageQueryResult> {
  const { program } = store.getState()
  try {
    return {
      id: pagePath,
      result: await readPageDataUtil(
        path.join(program.directory, `public`),
        pagePath
      ),
    }

Review comment (suggestion):
There is one more timing thing potentially to cover here - time between finishing query running and flushing fresh `page-data` file - with current code it's possible to fall into this return while `page-data` flush is pending for given page

Code after:
  IPageDataWithQueryResult,
  readPageData as readPageDataUtil,
} from "./page-data"

export async function getPageData(
  pagePath: string
): Promise<IPageDataWithQueryResult> {
  const { queries, pendingPageDataWrites } = store.getState()

  const query = queries.trackedQueries.get(pagePath)

  if (!query) {
    throw new Error(`Could not find query ${pagePath}`)
  }
  if (query.running !== 0) {
    return waitNextPageData(pagePath)
  }
  if (query.dirty !== 0) {
    emitter.emit(`QUERY_RUN_REQUESTED`, { pagePath })
    return waitNextPageData(pagePath)
  }
  if (pendingPageDataWrites.pagePaths.has(pagePath)) {
    return waitNextPageData(pagePath)
  }
  // Results are up-to-date
  return readPageData(pagePath)
}

async function waitNextPageData(
  pagePath: string
): Promise<IPageDataWithQueryResult> {
  return new Promise(resolve => {
    const listener = (data: IClearPendingPageDataWriteAction): void => {
      if (data.payload.page === pagePath) {
        emitter.off(`CLEAR_PENDING_PAGE_DATA_WRITE`, listener)
        resolve(readPageData(pagePath))
      }
    }
    emitter.on(`CLEAR_PENDING_PAGE_DATA_WRITE`, listener)
  })
}

async function readPageData(pagePath): Promise<IPageDataWithQueryResult> {
  const { program } = store.getState()

  try {
    return await readPageDataUtil(
      path.join(program.directory, `public`),
      pagePath
    )
  } catch (err) {
Repository: geekan/MetaGPT
File: metagpt/actions/write_tutorial.py (Python)

Code before:
        return await self._aask(prompt=prompt)


class SaveDocx(Action):
    """Action class for saving tutorial docx.

    Args:
        name: The name of the action.
    """

    def __init__(self, name: str = "", *args, **kwargs):
        super().__init__(name, *args, **kwargs)

    async def run(self, title: str, content: str, *args, **kwargs) -> str:
        """Execute the action to save the generated tutorial document to a Markdown file.

        Args:
            title: The title of tutorial.
            content: The total content of tutorial.

        Returns:
            The full filename of tutorial content.

        """
        current_time = datetime.now().strftime("%Y-%m-%d_%H-%M-%S")
        pathname = TUTORIAL_PATH / current_time
        pathname.mkdir(parents=True, exist_ok=True)
        filename = f"{pathname}/{title}.md"
        async with aiofiles.open(filename, mode="w", encoding="utf-8") as writer:
            await writer.write(content)
            logger.info(f"Successfully write docx: {filename}")
            return filename

Review comment (suggestion):
`SaveDocx` can works as a common MD saving operation, but here limit the `saving` path under `TUTORIAL_PATH`. So, the suggestion is to make it more universal or just save docx under `_react` like `https://github.com/geekan/MetaGPT/blob/main/metagpt/roles/researcher.py#L86C18-L86C18`

Code after:
    """Action class for writing tutorial content.

    Args:
        name: The name of the action.
        directory: The content to write.
        language: The language to output, default is "Chinese".
    """

    def __init__(self, name: str = "", directory: str = "", language: str = "Chinese", *args, **kwargs):
        super().__init__(name, *args, **kwargs)
        self.language = language
        self.directory = directory

    async def run(self, topic: str, *args, **kwargs) -> str:
        """Execute the action to write document content according to the directory and topic.

        Args:
            topic: The tutorial topic.

        Returns:
            The written tutorial content.
        """
        prompt = CONTENT_PROMPT.format(topic=topic, language=self.language, directory=self.directory)
        return await self._aask(prompt=prompt)
Repository: geekan/MetaGPT
File: metagpt/utils/file.py (Python)

Code before:
                    chunk = content[i:i + chunk_size]
                    await writer.write(chunk)
                    # Flush the buffer to ensure data is written immediately
                    await writer.flush()
                logger.info(f"Successfully write file: {full_path}")
                return full_path
        except Exception as e:
            logger.error(f"Error writing file: {e}")
            raise e

    @classmethod
    async def read(cls, file_path: Path, chunk_size: int = None) -> bytes:
        """Partitioning read the file content from the local specified path.

        Args:
            file_path: The full file name of file, such as "/data/test.txt".
            chunk_size: The size of each chunk in bytes (default is 64kb).

        Returns:
            The binary content of file.

        Raises:
            Exception: If an unexpected error occurs during the file reading process.
        """
        try:
            if not file_path.exists():
                raise FileNotFoundError(f"File not found, path is '{file_path}'")
            chunk_size = chunk_size or cls.CHUNK_SIZE
            async with aiofiles.open(file_path, mode="rb") as reader:
                content = bytes()
                while True:
                    chunk = await reader.read(chunk_size)
                    if not chunk:
                        break
                    content += chunk
                logger.info(f"Successfully read file, the size of file: {len(content)}")
                return content
        except Exception as e:
            logger.error(f"Error reading file: {e}")
            raise e



Review comment (suggestion):
I think this line is unnecessary. If the file path doesn't exist, it will raise a FileNotFoundError when using aiofiles.open.

Code after:
        except Exception as e:
            logger.error(f"Error writing file: {e}")
            raise e

    @classmethod
    async def read(cls, file_path: Path, chunk_size: int = None) -> bytes:
        """Partitioning read the file content from the local specified path.

        Args:
            file_path: The full file name of file, such as "/data/test.txt".
            chunk_size: The size of each chunk in bytes (default is 64kb).

        Returns:
            The binary content of file.

        Raises:
            Exception: If an unexpected error occurs during the file reading process.
        """
        try:
            chunk_size = chunk_size or cls.CHUNK_SIZE
            async with aiofiles.open(file_path, mode="rb") as reader:
                chunks = list()
                while True:
                    chunk = await reader.read(chunk_size)
                    if not chunk:
                        break
                    chunks.append(chunk)
                content = b''.join(chunks)
                logger.debug(f"Successfully read file, the path of file: {file_path}")
                return content
        except Exception as e:
            logger.error(f"Error reading file: {e}")
            raise e
Repository: geekan/MetaGPT
File: metagpt/utils/file.py (Python)

Code before:

    @classmethod
    async def read(cls, file_path: Path, chunk_size: int = None) -> bytes:
        """Partitioning read the file content from the local specified path.

        Args:
            file_path: The full file name of file, such as "/data/test.txt".
            chunk_size: The size of each chunk in bytes (default is 64kb).

        Returns:
            The binary content of file.

        Raises:
            Exception: If an unexpected error occurs during the file reading process.
        """
        try:
            if not file_path.exists():
                raise FileNotFoundError(f"File not found, path is '{file_path}'")
            chunk_size = chunk_size or cls.CHUNK_SIZE
            async with aiofiles.open(file_path, mode="rb") as reader:
                content = bytes()
                while True:
                    chunk = await reader.read(chunk_size)
                    if not chunk:
                        break
                    content += chunk
                logger.info(f"Successfully read file, the size of file: {len(content)}")
                return content
        except Exception as e:
            logger.error(f"Error reading file: {e}")
            raise e



Review comment (suggestion):
Use [bytes.join()](https://docs.python.org/3/library/stdtypes.html#bytes.join) to replace repeated concatenation.
See the note 6 in https://docs.python.org/3/library/stdtypes.html#common-sequence-operations


Code after:
            file_path: The full file name of file, such as "/data/test.txt".
            chunk_size: The size of each chunk in bytes (default is 64kb).

        Returns:
            The binary content of file.

        Raises:
            Exception: If an unexpected error occurs during the file reading process.
        """
        try:
            chunk_size = chunk_size or cls.CHUNK_SIZE
            async with aiofiles.open(file_path, mode="rb") as reader:
                chunks = list()
                while True:
                    chunk = await reader.read(chunk_size)
                    if not chunk:
                        break
                    chunks.append(chunk)
                content = b''.join(chunks)
                logger.debug(f"Successfully read file, the path of file: {file_path}")
                return content
        except Exception as e:
            logger.error(f"Error reading file: {e}")
            raise e
Repository: geekan/MetaGPT
File: metagpt/utils/dependency_file.py (Python)

Code before:
                s = re.sub(r"\\+", "/", s)  # Compatible with windows path
                relative_paths.append(s)

            self._dependencies[skey] = relative_paths
        elif skey in self._dependencies:
            del self._dependencies[skey]

        if persist:
            await self.save()

    async def get(self, filename: Path | str, persist=True):
        """Get dependencies for a file asynchronously.

        :param filename: The filename or path.
        :param persist: Whether to load dependencies from the file immediately.
        :return: A set of dependencies.
        """
        if persist:
            await self.load()

        root = self._filename.parent
        try:
            key = Path(filename).relative_to(root)
        except ValueError:
            key = filename
        key = re.sub(r"\\+", "/", str(key))
        return set(self._dependencies.get(key, {}))

    def delete_file(self):
        """Delete the dependency file."""
        self._filename.unlink(missing_ok=True)

    @property
    def exists(self):
        """Check if the dependency file exists."""
        return self._filename.exists()


Review comment (suggestion):
1. use `pathlib.path(r'foo\bar').as_posix()` would be better.
2. it's too many defense in this file. I see 4 `re.sub(r"\\+"`, it's not necessary. Just delete 3 and leave one path.as_posix()

Code after:
                s = re.sub(r"\\+", "/", s)  # Compatible with windows path
                relative_paths.append(s)

            self._dependencies[skey] = relative_paths
        elif skey in self._dependencies:
            del self._dependencies[skey]

        if persist:
            await self.save()

    async def get(self, filename: Path | str, persist=True):
        """Get dependencies for a file asynchronously.

        :param filename: The filename or path.
        :param persist: Whether to load dependencies from the file immediately.
        :return: A set of dependencies.
        """
        if persist:
            await self.load()

        root = self._filename.parent
        try:
            key = Path(filename).relative_to(root).as_posix()
        except ValueError:
            key = filename
        return set(self._dependencies.get(str(key), {}))

    def delete_file(self):
        """Delete the dependency file."""
        self._filename.unlink(missing_ok=True)

    @property
    def exists(self):
        """Check if the dependency file exists."""
        return self._filename.exists()
Repository: geekan/MetaGPT
File: examples/search_with_specific_engine.py (Python)

Code before:
#!/usr/bin/env python
# -*- coding: utf-8 -*-
"""
"""
import asyncio

from metagpt.config2 import Config
from metagpt.roles import Searcher
from metagpt.tools.search_engine import SearchEngine, SearchEngineType


async def main():
    question = "What are the most interesting human facts?"

    search = Config.default().search
    kwargs = {"api_key": search.api_key, "cse_id": search.cse_id, "proxy": None}

    if search.api_type == SearchEngineType.DIRECT_GOOGLE:
        # Google API
        await Searcher(
            search_engine=SearchEngine(engine=SearchEngineType.DIRECT_GOOGLE, **kwargs)
        ).run(question)
    elif search.api_type == SearchEngineType.SERPER_GOOGLE:
        # Serper API
        await Searcher(
            search_engine=SearchEngine(engine=SearchEngineType.SERPER_GOOGLE, **kwargs)
        ).run(question)
    elif search.api_type == SearchEngineType.SERPAPI_GOOGLE:
        # SerpAPI
        await Searcher(
            search_engine=SearchEngine(engine=SearchEngineType.SERPAPI_GOOGLE, **kwargs)
        ).run(question)
    else:
        # DDG API
        await Searcher(
            search_engine=SearchEngine(engine=SearchEngineType.DUCK_DUCK_GO, **kwargs)
        ).run(question)


if __name__ == "__main__":
    asyncio.run(main())


Review comment (performance):
You can replace these if-else statements with a dict to reduce complexity, like this:
```python
mappings = {
    SearchEngineType.DIRECT_GOOGLE: {
        "api_key": search.api_key, 
        "cse_id": search.cse_id, 
        "proxy": None, 
        "engine":SearchEngineType.DIRECT_GOOGLE}, 
     ...}
args = mappings.get(search.api_type)
search_engine=SearchEngine(**args)
```


Code after:
#!/usr/bin/env python
# -*- coding: utf-8 -*-
"""
"""
import asyncio

from metagpt.config2 import Config
from metagpt.roles import Searcher
from metagpt.tools.search_engine import SearchEngine


async def main():
    question = "What are the most interesting human facts?"

    search = Config.default().search
    kwargs = {"api_key": search.api_key, "cse_id": search.cse_id, "proxy": None}
    await Searcher(search_engine=SearchEngine(engine=search.api_type, **kwargs)).run(question)


if __name__ == "__main__":
    asyncio.run(main())
Repository: geekan/MetaGPT
File: examples/search_with_specific_engine.py (Python)

Code before:
#!/usr/bin/env python
# -*- coding: utf-8 -*-
"""
"""
import asyncio

from metagpt.config2 import Config
from metagpt.roles import Searcher
from metagpt.tools.search_engine import SearchEngine


async def main():
    question = "What are the most interesting human facts?"

    search = Config.default().search
    kwargs = {"api_key": search.api_key, "cse_id": search.cse_id, "proxy": None, "params": search.params}
    await Searcher(search_engine=SearchEngine(engine=search.api_type, **kwargs)).run(question)


if __name__ == "__main__":
    asyncio.run(main())


Review comment (suggestion):
```python
    kwargs = search.model_dump()
    await Searcher(search_engine=SearchEngine(**kwargs)).run(question)
```
Using the above code should have the same effect.

Code after:
#!/usr/bin/env python
# -*- coding: utf-8 -*-
"""
"""
import asyncio

from metagpt.config2 import Config
from metagpt.roles import Searcher
from metagpt.tools.search_engine import SearchEngine


async def main():
    question = "What are the most interesting human facts?"

    search = Config.default().search
    kwargs = search.model_dump()
    await Searcher(search_engine=SearchEngine(engine=search.api_type, **kwargs)).run(question)


if __name__ == "__main__":
    asyncio.run(main())
Repository: getumbrel/llama-gpt
File: api/run.sh (Shell)

Code before:
    echo "Please set the MODEL_FILE environment variable"
    exit 1
fi

if [ -z "$MODEL_DOWNLOAD_URL" ]
then
    echo "Please set the MODEL_DOWNLOAD_URL environment variable"
    exit 1
fi


if [ ! -f $MODEL ]; then
    echo "Model file not found. Downloading..."
    curl -L -o $MODEL $MODEL_DOWNLOAD_URL
else
    echo "$MODEL model found."
fi

# Get the number of available threads on the system
n_threads=$(grep -c ^processor /proc/cpuinfo)

# Define context window
n_ctx=4096

# Offload everything to CPU
n_gpu_layers=0

# Define batch size
n_batch=2096
# If total RAM is less than 8GB, set batch size to 1024
total_ram=$(cat /proc/meminfo | grep MemTotal | awk '{print $2}')
if [ $total_ram -lt 8000000 ]; then
    n_batch=1024
fi

echo "Initializing server with:"
echo "Batch size: $n_batch"
echo "Number of CPU threads: $n_threads"
echo "Number of GPU layers: $n_gpu_layers"
echo "Context window: $n_ctx"

exec python3 -m llama_cpp.server --n_ctx $n_ctx --n_threads $n_threads --n_gpu_layers $n_gpu_layers --n_batch $n_batch


Review comment (suggestion):
```suggestion

make build
```

I couldn't get it to work without a build.

Code after:
    echo "Please set the MODEL_FILE environment variable"
    exit 1
fi

if [ -z "$MODEL_DOWNLOAD_URL" ]
then
    echo "Please set the MODEL_DOWNLOAD_URL environment variable"
    exit 1
fi

# check if curl is installed

if ! [ -x "$(command -v curl)" ]; then
    echo "curl is not installed. Installing..."
    apt-get update --yes --quiet
    apt-get install --yes --quiet curl
fi


if [ ! -f $MODEL ]; then
    echo "Model file not found. Downloading..."
    curl -L -o $MODEL $MODEL_DOWNLOAD_URL
else
    echo "$MODEL model found."
fi

make build
# Get the number of available threads on the system
n_threads=$(grep -c ^processor /proc/cpuinfo)

# Define context window
n_ctx=4096

# Offload everything to CPU
n_gpu_layers=0

# Define batch size
n_batch=2096
# If total RAM is less than 8GB, set batch size to 1024
total_ram=$(cat /proc/meminfo | grep MemTotal | awk '{print $2}')
if [ $total_ram -lt 8000000 ]; then
    n_batch=1024
fi

echo "Initializing server with:"
echo "Batch size: $n_batch"
echo "Number of CPU threads: $n_threads"
echo "Number of GPU layers: $n_gpu_layers"
echo "Context window: $n_ctx"

exec python3 -m llama_cpp.server --n_ctx $n_ctx --n_threads $n_threads --n_gpu_layers $n_gpu_layers --n_batch $n_batch