ronantakizawa/codereview-bench huggingface dataset:
Repository: pedroslopez/whatsapp-web.js
File: src/Client.js (JavaScript)

Code before:
            return flag;
        }, flag);
    }

    /**
     * Get user device count by ID
     * Each WaWeb Connection counts as one device, and the phone (if exists) counts as one
     * So for a non-enterprise user with one WaWeb connection it should return "2"
     * @param {string} contactId
     * @returns {number}
     */
    async getContactDeviceCount(contactId) {
        let devices = await window.Store.DeviceList.getDeviceIds([window.Store.WidFactory.createWid(contactId)]);
        if(devices && devices.length  && devices[0] != null && typeof devices[0].devices == 'object'){
            return devices[0].devices.length;
        }
        return 0;
    }

    /**
     * Sync history conversation
     * @param {string} chatId
     * @return {boolean} true/false
     */
    async syncHistory(chatId) {
        return await this.pupPage.evaluate(async (chatId) => {
            const chat = await window.WWebJS.getChat(chatId);
            if (chat.endOfHistoryTransferType == window.Store.HistoryStates.COMPLETE_BUT_MORE_MESSAGES_REMAIN_ON_PRIMARY) {
                window.Store.HistorySync.sendPeerDataOperationRequest(window.require("WAWebProtobufsE2E.pb").Message$PeerDataOperationRequestType.HISTORY_SYNC_ON_DEMAND, {
                    chatId: chat.id
                });
                return true;
            }
            return false;
        }, chatId);
    }
}

module.exports = Client;

Review comment (suggestion):
We don't need to use `await` here since the method already returns `Promise`.
```suggestion
        return this.pupPage.evaluate(async (chatId) => {
```

Code after:
            return flag;
        }, flag);
    }

    /**
     * Get user device count by ID
     * Each WaWeb Connection counts as one device, and the phone (if exists) counts as one
     * So for a non-enterprise user with one WaWeb connection it should return "2"
     * @param {string} contactId
     * @returns {number}
     */
    async getContactDeviceCount(contactId) {
        let devices = await window.Store.DeviceList.getDeviceIds([window.Store.WidFactory.createWid(contactId)]);
        if(devices && devices.length  && devices[0] != null && typeof devices[0].devices == 'object'){
            return devices[0].devices.length;
        }
        return 0;
    }

    /**
     * Sync chat history conversation
     * @param {string} chatId
     * @return {Promise<boolean>} True if operation completed successfully, false otherwise.
     */
    async syncHistory(chatId) {
        return await this.pupPage.evaluate(async (chatId) => {
            const chat = await window.WWebJS.getChat(chatId);
            if (chat.endOfHistoryTransferType === 0) {
                await window.Store.HistorySync.sendPeerDataOperationRequest(3, {
                    chatId: chat.id
                });
                return true;
            }
            return false;
        }, chatId);
    }
}

module.exports = Client;
Repository: pedroslopez/whatsapp-web.js
File: src/structures/Broadcast.js (JavaScript)

Code before:
         */
        this.id = data.id;

        /**
         * Unix timestamp of last status
         * @type {number}
         */
        this.timestamp = data.t;

        /**
         * Number of available statuses
         * @type {number}
         */
        this.totalCount = data.totalCount;

        /**
         * Number of not viewed
         * @type {number}
         */
        this.unreadCount = data.unreadCount;

        /**
         * Messages statuses
         * @type {Message[]}
         */
        this.msgs = data.msgs.map(msg => new Message(super.client, msg));

        return super._patch(data);
    }

    /**
     * Returns the Chat this message was sent in
     * @returns {Promise<Chat>}
     */
    getChat() {
        return this.client.getChatById(this.id._serialized);
    }

    /**
     * Returns the Contact this message was sent from
     * @returns {Promise<Contact>}
     */
    getContact() {
        return this.client.getContactById(this.id._serialized);
    }

}

module.exports = Broadcast;


Review comment (suggestion):
Please fix this, `super.client` can possibly throw an error, see more [here](https://github.com/pedroslopez/whatsapp-web.js/pull/3296).

```suggestion
        this.msgs = data.msgs.map(msg => new Message(this.client, msg));
```

Code after:
         */
        this.id = data.id;

        /**
         * Unix timestamp of last status
         * @type {number}
         */
        this.timestamp = data.t;

        /**
         * Number of available statuses
         * @type {number}
         */
        this.totalCount = data.totalCount;

        /**
         * Number of not viewed
         * @type {number}
         */
        this.unreadCount = data.unreadCount;

        /**
         * Messages statuses
         * @type {Message[]}
         */
        this.msgs = data.msgs.map(msg => new Message(this.client, msg));

        return super._patch(data);
    }

    /**
     * Returns the Chat this message was sent in
     * @returns {Promise<Chat>}
     */
    getChat() {
        return this.client.getChatById(this.id._serialized);
    }

    /**
     * Returns the Contact this message was sent from
     * @returns {Promise<Contact>}
     */
    getContact() {
        return this.client.getContactById(this.id._serialized);
    }

}

module.exports = Broadcast;
Repository: pedroslopez/whatsapp-web.js
File: example.js (JavaScript)

Code before:
const { Client, Location, Poll, List, Buttons, LocalAuth } = require('./index');

const client = new Client({
    authStrategy: new LocalAuth(),
    // proxyAuthentication: { username: 'username', password: 'password' },
    /** Check https://github.com/pedroslopez/whatsapp-web.js/pull/3325 for full explanation. */
    // deviceName: 'TEST',
    // browserName: 'Firefox', // valid value are: 'Chrome' | 'Firefox' | 'IE' | 'Opera' | 'Safari' | 'Edge'
    puppeteer: { 
        // args: ['--proxy-server=proxy-server-that-requires-authentication.example.com'],
        headless: false,
    }
});

// client initialize does not finish at ready now.
client.initialize();

client.on('loading_screen', (percent, message) => {
    console.log('LOADING SCREEN', percent, message);
});

// Pairing code only needs to be requested once
let pairingCodeRequested = false;
client.on('qr', async (qr) => {
    // NOTE: This event will not be fired if a session is specified.
    console.log('QR RECEIVED', qr);

    // paiuting code example
    const pairingCodeEnabled = false;
    if (pairingCodeEnabled && !pairingCodeRequested) {
        const pairingCode = await client.requestPairingCode('96170100100'); // enter the target phone number

Review comment (suggestion):
The explanation and details should be either in example itself or in readme.md.
Imo, it's not a good practice to have a link to VCS (especially in github). 

We've seen many github repos got removed and we'll lose important notes :) 

Code after:
const { Client, Location, Poll, List, Buttons, LocalAuth } = require('./index');

const client = new Client({
    authStrategy: new LocalAuth(),
    // proxyAuthentication: { username: 'username', password: 'password' },
    /**
     * This option changes the browser name from defined in user agent to custom.
     */
    // deviceName: 'Your custom name',
    /**
     * This option changes browser type from defined in user agent to yours. It affects the browser icon
     * that is displayed in 'linked devices' section.
     * Valid value are: 'Chrome' | 'Firefox' | 'IE' | 'Opera' | 'Safari' | 'Edge'.
     * If another value is provided, the browser icon in 'linked devices' section will be gray.
     */
    // browserName: 'Firefox',
    puppeteer: { 
        // args: ['--proxy-server=proxy-server-that-requires-authentication.example.com'],
        headless: false,
    }
});

// client initialize does not finish at ready now.
client.initialize();

client.on('loading_screen', (percent, message) => {
    console.log('LOADING SCREEN', percent, message);
});

// Pairing code only needs to be requested once
let pairingCodeRequested = false;
Repository: phoboslab/wipeout-rewrite
File: src/platform_sdl.c (C)

Code before:
	char *path = strcat(strcpy(temp_path, path_userdata), name);
	return file_store(path, bytes, len);
}

#if defined(RENDERER_GL) // ----------------------------------------------------
	#define PLATFORM_WINDOW_FLAGS SDL_WINDOW_OPENGL
	SDL_GLContext platform_gl;

	void platform_video_init() {
		#if defined(USE_GLES2)
			SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_ES);
			SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2);
			SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1);
		#endif

		platform_gl = SDL_GL_CreateContext(window);
		SDL_GL_SetSwapInterval(1);
	}

	void platform_prepare_frame() {
		// nothing
	}

	void platform_video_cleanup() {
		SDL_GL_DeleteContext(platform_gl);
		SDL_DestroyWindow(window);	
	}

	void platform_end_frame() {
		SDL_GL_SwapWindow(window);
	}

	vec2i_t platform_screen_size() {
		int width, height;
		SDL_GL_GetDrawableSize(window, &width, &height);
		return vec2i(width, height);
	}


#elif defined(RENDERER_SOFTWARE) // ----------------------------------------------
	#define PLATFORM_WINDOW_FLAGS 0
	static SDL_Renderer *renderer;
	static SDL_Texture *screenbuffer = NULL;
	static void *screenbuffer_pixels = NULL;
	static int screenbuffer_pitch;
	static vec2i_t screenbuffer_size = vec2i(0, 0);
	static vec2i_t screen_size = vec2i(0, 0);


	void platform_video_init() {
		renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);

Review comment (suggestion):
`SDL_DestroyWindow()` should be in `main()` since the window is also created in `main()` (not in `platform_video_init()`)

Code after:
	char *path = strcat(strcpy(temp_path, path_userdata), name);
	return file_store(path, bytes, len);
}

#if defined(RENDERER_GL) // ----------------------------------------------------
	#define PLATFORM_WINDOW_FLAGS SDL_WINDOW_OPENGL
	SDL_GLContext platform_gl;

	void platform_video_init() {
		#if defined(USE_GLES2)
			SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_ES);
			SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2);
			SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1);
		#endif

		platform_gl = SDL_GL_CreateContext(window);
		SDL_GL_SetSwapInterval(1);
	}

	void platform_prepare_frame() {
		// nothing
	}

	void platform_video_cleanup() {
		SDL_GL_DeleteContext(platform_gl);
	}

	void platform_end_frame() {
		SDL_GL_SwapWindow(window);
	}

	vec2i_t platform_screen_size() {
		int width, height;
		SDL_GL_GetDrawableSize(window, &width, &height);
		return vec2i(width, height);
	}


#elif defined(RENDERER_SOFTWARE) // ----------------------------------------------
	#define PLATFORM_WINDOW_FLAGS 0
	static SDL_Renderer *renderer;
	static SDL_Texture *screenbuffer = NULL;
	static void *screenbuffer_pixels = NULL;
	static int screenbuffer_pitch;
	static vec2i_t screenbuffer_size = vec2i(0, 0);
	static vec2i_t screen_size = vec2i(0, 0);


	void platform_video_init() {
		renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);
	}
Repository: ragapp/ragapp
File: admin/client/config.ts (TypeScript)

Code before:
export async function updateConfig(
  data: ConfigFormType,
): Promise<ConfigFormType> {
  // Ignore configured attribute
  const { configured, ...updateValues } = data;

  const res = await fetch(`${getBaseURL()}/api/management/config`, {
    method: "POST",
    headers: {
      "Content-Type": "application/json",
    },
    body: JSON.stringify(updateValues),
  });
  if (!res.ok) {
    const error = await res.text();
    throw new Error(error);
  }
  return (await res.json()).data as ConfigFormType;
}

export async function fetchModels(
  provider: string,
  providerUrl: string,
): Promise<string[]> {
  const res = await fetch(
    `${getBaseURL()}/api/management/config/models?provider=${provider}&provider_url=${providerUrl}`,
  );
  if (!res.ok) {
    const error = await res.text();
    throw new Error(error);
  }
  const data = await res.json();
  return data;
}


Review comment (suggestion):
don't you need `encodeURIComponent` to encode the GET parameter?
```suggestion
    `${getBaseURL()}/api/management/config/models?provider=${provider}&provider_url=${encodeURIComponent(providerUrl)}`,
```

Code after:
export async function updateConfig(
  data: ConfigFormType,
): Promise<ConfigFormType> {
  // Ignore configured attribute
  const { configured, ...updateValues } = data;

  const res = await fetch(`${getBaseURL()}/api/management/config`, {
    method: "POST",
    headers: {
      "Content-Type": "application/json",
    },
    body: JSON.stringify(updateValues),
  });
  if (!res.ok) {
    const error = await res.text();
    throw new Error(error);
  }
  return (await res.json()).data as ConfigFormType;
}

export async function fetchModels(
  provider: string,
  providerUrl: string,
): Promise<string[]> {
  const res = await fetch(
    `${getBaseURL()}/api/management/config/models?provider=${provider}&provider_url=${encodeURIComponent(providerUrl)}`,
  );
  if (!res.ok) {
    const error = await res.text();
    throw new Error(error);
  }
  const data = await res.json();
  return data;
}
Repository: ragapp/ragapp
File: admin/sections/config/chat.tsx (TypeScript)

Code before:
  return ref;
}

export const ChatConfig = ({
  form,
  isSubmitting,
  addCallback,
}: {
  form: any;
  isSubmitting: boolean;
  addCallback: any;
}) => {
  // To detect the change of conversation starters
  const conversationStarters = form.watch("conversation_starters");
  const initialConversationStartersRef = useInitialValue(conversationStarters);

  async function onClickSubmit() {
    if (
      JSON.stringify(conversationStarters) !==
      JSON.stringify(initialConversationStartersRef.current)
    ) {
      initialConversationStartersRef.current = conversationStarters;
      addCallback((prev: any) => [...prev, reloadDemoChat]);
    }
  }

  return (
    <ExpandableSection
      title={"Chat Config"}
      description="Config how the chatbot behaves and interacts with the user"
      open={true}
    >
      <FormField
        control={form.control}
        name="system_prompt"
        render={({ field }) => (
          <FormItem>
            <FormLabel>Custom Prompt</FormLabel>
            <FormControl>
              <Textarea rows={3} {...field} />
            </FormControl>
            <FormDescription>
              Use system prompt to define the responsibilities and behaviors of
              the assistant.
            </FormDescription>
          </FormItem>
        )}
      />
      <FormField
        control={form.control}
        name="conversation_starters"

Review comment (bug):
Should refactor the whole code here, it's not good pattern in React:
- do not use `any`
- do not use `useRef` to manage initial value
- compare by JSON.stringify may lead to bug

Code after:
    >
      <FormField
        control={form.control}
        name="system_prompt"
        render={({ field }) => (
          <FormItem>
            <FormLabel>Custom Prompt</FormLabel>
            <FormControl>
              <Textarea rows={3} {...field} />
            </FormControl>
            <FormDescription>
              Use system prompt to define the responsibilities and behaviors of
              the assistant.
            </FormDescription>
          </FormItem>
        )}
      />
      <FormField
        control={form.control}
        name="conversation_starters"
        render={({ field }) => (
          <FormItem className="pt-4">
            <FormLabel>Conversation questions</FormLabel>
            <FormControl>
              <MultiInput {...field} />
            </FormControl>
            <FormDescription>
              Add suggested questions to help users start a conversation with
              the app.
            </FormDescription>
            <FormMessage />
          </FormItem>
        )}
      />
      <div className="mt-4">
        <SubmitButton isSubmitting={isSubmitting} />
      </div>
    </ExpandableSection>
  );
};
Repository: ragapp/ragapp
File: src/models/loader.py (Python)

Code before:
    use_llama_parse: bool = Field(
        default=False,
        description="Use Llama Parser to parse the file.",
    )
    llama_cloud_api_key: str | None = Field(
        default=None,
        description="API key for Llama Parse.",
    )

    def __init__(self, **data):
        env_api_key = os.getenv("LLAMA_CLOUD_API_KEY")
        if env_api_key:
            data["llama_cloud_api_key"] = env_api_key
        super().__init__(**data)

    def update_env_api_key(self):
        """
        Update the environment variable for the API key.
        """
        import dotenv

        if self.llama_cloud_api_key:
            # Update runtime environment variable
            os.environ["LLAMA_CLOUD_API_KEY"] = self.llama_cloud_api_key
            # Update .env file
            dotenv.set_key(
                ENV_FILE_PATH, "LLAMA_CLOUD_API_KEY", self.llama_cloud_api_key
            )

    def to_config_dict(self):
        """
        Convert the loader configuration to a dictionary.
        """
        return {
            "use_llama_parse": self.use_llama_parse,
        }


LoaderConfig = Union[FileLoader]
SupportedLoaders = Literal["file"]


Review comment (suggestion):
reading this, i think it makes sense to encapsulate the API keys that are needed by loaders to the file loader model.
similar we might want to encapsulate env keys for tool configuration in the tool model later.

but one question: how does this interfere with the env config model? we have to ensure that both models (env config and file loader) can be serialized and deserialized independently without impact. e.g. if the env config is serialized we don't want `LLAMA_CLOUD_API_KEY` to be overwritten. 

Code after:
    use_llama_parse: bool = Field(
        default=False,
        description="Use Llama Parser to parse the file.",
    )
    llama_cloud_api_key: str | None = Field(
        default=None,
        description="API key for Llama Parse.",
    )

    def __init__(self, **data):
        env_api_key = os.getenv("LLAMA_CLOUD_API_KEY")
        if env_api_key and "llama_cloud_api_key" not in data:
            data["llama_cloud_api_key"] = env_api_key
        super().__init__(**data)

    def update_env_api_key(self):
        """
        Update the environment variable for the API key.
        """
        import dotenv

        if self.llama_cloud_api_key:
            # Update runtime environment variable
            os.environ["LLAMA_CLOUD_API_KEY"] = self.llama_cloud_api_key
            # Update .env file
            dotenv.set_key(
                ENV_FILE_PATH, "LLAMA_CLOUD_API_KEY", self.llama_cloud_api_key
            )

    def to_config_dict(self):
        """
        Convert the loader configuration to a dictionary.
        """
        return {
            "use_llama_parse": self.use_llama_parse,
        }


LoaderConfig = Union[FileLoader]
SupportedLoaders = Literal["file"]
Repository: ragapp/ragapp
File: admin/sections/config/chat.tsx (TypeScript)

Code before:
  FormItem,
  FormLabel,
  FormMessage,
} from "@/components/ui/form";
import { Textarea } from "@/components/ui/textarea";
import { toast } from "@/components/ui/use-toast";
import { zodResolver } from "@hookform/resolvers/zod";
import { useEffect } from "react";
import { useForm } from "react-hook-form";
import { useMutation, useQuery } from "react-query";

export const ChatConfig = ({}: {}) => {
  const form = useForm<ChatConfigFormType>({
    resolver: zodResolver(ChatConfigSchema),
  });

  // Use query to fetch the chat config
  // TODO: show isLoading state, e.g. a spinner in the exapandable section component
  const { data, error, isLoading } = useQuery("chatConfig", getChatConfig, {
    refetchOnWindowFocus: false,
  });
  const { mutate: updateConfig, updateError } = useMutation(updateChatConfig, {
    onError: (error: unknown) => {
      // TODO: form.reset(data) is just resetting to the last value, but a failure in the API
      // call doesn't guarantee that the data is still the same.
      // @Lee can we guarantee this? if not we should probably refetch the data
      // see https://tanstack.com/query/v4/docs/framework/react/guides/mutations#mutation-side-effects
      form.reset(data);
      console.error(error);
      toast({
        title: "Failed to update chat config",
        variant: "destructive",
      });
    },
  });

  useEffect(() => {
    if (data) {
      form.reset(data);
    }
  }, [data, form]);

  async function handleSubmit() {
    updateConfig(form.getValues());
  }

  return (
    <ExpandableSection
      name="chat-config"
      title={"Chat Config"}
      description="Config how the chatbot behaves and interacts with the user"

Review comment (suggestion):
Yeah, the chat config doesn't run any data tasks, so it's fine to keep the config as is, if there's an error.

Code after:
  FormItem,
  FormLabel,
  FormMessage,
} from "@/components/ui/form";
import { Textarea } from "@/components/ui/textarea";
import { toast } from "@/components/ui/use-toast";
import { zodResolver } from "@hookform/resolvers/zod";
import { useForm } from "react-hook-form";
import { useMutation, useQuery } from "react-query";

export const ChatConfig = ({}: {}) => {
  const {
    data,
    isLoading: isFetching,
    refetch,
    isRefetching,
  } = useQuery("chatConfig", getChatConfig, {
    refetchOnWindowFocus: false,
  });
  const form = useForm<ChatConfigFormType>({
    resolver: zodResolver(ChatConfigSchema),
    defaultValues: {
      system_prompt: "",
      conversation_starters: [],
    },
    values: data,
  });

  const { mutate: updateConfig, isLoading: isSubmitting } = useMutation(
    updateChatConfig,
    {
      onError: (error: unknown) => {
        console.error(error);
        toast({
          title: "Failed to update chat config",
          variant: "destructive",
        });
        form.reset();
      },
      onSuccess: () => {
        refetch();
      },
    },
  );

  async function handleSubmit() {
    updateConfig(form.getValues());
  }

  const isLoading = isFetching || isRefetching || isSubmitting;