ronantakizawa/codereview-bench huggingface dataset:
Repository: gradio-app/gradio
File: js/core/src/dependency.ts (TypeScript)

Code before:
						data: data_payload,
						event_data: event_meta.event_data,
						trigger_id: target_id
					});
					const dep_submission = await dep.run(
						this.client,
						data_payload,
						event_meta.event_data,
						target_id
					);

					if (dep_submission.type === "void") {
						unset_args.forEach((fn) => fn());
					} else if (dep_submission.type === "data") {
						await this.handle_data(dep.outputs, dep_submission.data);
						unset_args.forEach((fn) => fn());
					} else {
						let stream_state: "open" | "closed" | "waiting" | null = null;

						if (
							dep.connection_type === "stream" &&
							!this.submissions.has(dep.id)
						) {
							stream_state = "waiting";
						}

						this.submissions.set(dep.id, dep_submission.data);
						let index = 0;
						// fn for this?
						submit_loop: for await (const result of dep_submission.data) {
							if (index === 0) {
								// Clear out previously set validation errors
								dep.inputs.forEach((input_id) => {
									this.update_state_cb(
										input_id,
										{
											loading_status: {
												validation_error: null
											}
										},
										false
									);
								});
							}
							index += 1;
							if (result === null) continue;
							if (result.type === "data") {
								await this.handle_data(dep.outputs, result.data);
							}
							if (result.type === "status") {
								if (

Review comment (bug):
added these awaits because otherwise, if there was a .then that triggered another function, and it used the outputs of the first dependency as inputs, the gathered data would not reflect the updated data sometimes, annoying race condition

Code after:
							fn_index: dep.id,
							data: data_payload,
							event_data: event_meta.event_data
						};
						submission!.send_chunk(payload);
						unset_args.forEach((fn) => fn());
						continue;
					}

					this.add_to_api_calls({
						fn_index: dep.id,
						data: data_payload,
						event_data: event_meta.event_data,
						trigger_id: target_id
					});
					const dep_submission = await dep.run(
						this.client,
						data_payload,
						event_meta.event_data,
						target_id
					);

					if (dep_submission.type === "void") {
						unset_args.forEach((fn) => fn());
					} else if (dep_submission.type === "data") {
						await this.handle_data(dep.outputs, dep_submission.data);
						unset_args.forEach((fn) => fn());
					} else {
						let stream_state: "open" | "closed" | "waiting" | null = null;

						if (
							dep.connection_type === "stream" &&
							!this.submissions.has(dep.id)
						) {
							stream_state = "waiting";
						}

						this.submissions.set(dep.id, dep_submission.data);
						let index = 0;
						// fn for this?
						submit_loop: for await (const result of dep_submission.data) {
							if (index === 0) {
								// Clear out previously set validation errors
								dep.inputs.forEach((input_id) => {
									this.update_state_cb(
										input_id,
										{
											loading_status: {
												validation_error: null
											}
										},
Repository: gradio-app/gradio
File: gradio/component_meta.py (Python)

Code before:
                raise ComponentDefinitionError(
                    f"{name} or its base classes must define an EVENTS list. "
                    "If no events are supported, set it to an empty list."
                )
        events = attrs.get("EVENTS", [])
        if not all(isinstance(e, (str, EventListener)) for e in events):
            raise ComponentDefinitionError(
                f"All events for {name} must either be an string or an instance "
                "of EventListener."
            )
        new_events = []
        for event in events:
            trigger = (
                event
                if isinstance(event, EventListener)
                else EventListener(event_name=event)
            ).copy()
            new_events.append(trigger)
            trigger.set_doc(component=name)
            attrs[event] = trigger.listener
        if "EVENTS" in attrs:
            attrs["EVENTS"] = new_events
        component_class = super().__new__(cls, name, bases, attrs)

        with no_raise_exception():
            create_or_modify_pyi(component_class, name, events)
        return component_class


Review comment (suggestion):
This will raise an error for custom HTML components (and it's not really needed since it's just pyi files for IDEs which are not relevant for custom HTML components)

Code after:
            for base in bases:
                if hasattr(base, "EVENTS"):
                    found = True
                    break
            if not found:
                raise ComponentDefinitionError(
                    f"{name} or its base classes must define an EVENTS list. "
                    "If no events are supported, set it to an empty list."
                )
        events = attrs.get("EVENTS", [])
        if not all(isinstance(e, (str, EventListener)) for e in events):
            raise ComponentDefinitionError(
                f"All events for {name} must either be an string or an instance "
                "of EventListener."
            )
        new_events = []
        for event in events:
            trigger = (
                event
                if isinstance(event, EventListener)
                else EventListener(event_name=event)
            ).copy()
            new_events.append(trigger)
            trigger.set_doc(component=name)
            attrs[event] = trigger.listener
        if "EVENTS" in attrs:
            attrs["EVENTS"] = new_events
        component_class = super().__new__(cls, name, bases, attrs)

        create_or_modify_pyi(component_class, name, events)
        return component_class
Repository: gradio-app/gradio
File: js/textbox/shared/Textbox.svelte (Svelte)

Code before:
		<IconButtonWrapper>
			<IconButton
				Icon={copied ? Check : Copy}
				on:click={handle_copy}
				label={copied ? "Copied" : "Copy"}
			/>
		</IconButtonWrapper>
	{/if}
	<BlockTitle show_label={validation_error ? true : show_label} {info}
		>{label}
		{#if validation_error}
			<div class="validation-error">{validation_error}</div>
		{/if}
	</BlockTitle>

	<div class="input-container">
		{#if lines === 1 && _max_lines === 1}
			{#if type === "text"}
				<input
					data-testid="textbox"
					type="text"
					class="scroll-hide"
					dir={rtl ? "rtl" : "ltr"}
					bind:value
					bind:this={el}
					placeholder={disabled ? "" : placeholder}
					{disabled}
					{autofocus}
					maxlength={max_length}
					on:keypress={handle_keypress}
					on:blur
					on:select={handle_select}
					on:focus
					class:validation-error={validation_error}
					style={text_align ? "text-align: " + text_align : ""}
					autocapitalize={html_attributes?.autocapitalize}
					autocorrect={html_attributes?.autocorrect}
					spellcheck={html_attributes?.spellcheck}
					autocomplete={html_attributes?.autocomplete}
					tabindex={html_attributes?.tabindex}
					enterkeyhint={html_attributes?.enterkeyhint}
					lang={html_attributes?.lang}
				/>
			{:else if type === "password"}
				<input
					data-testid="password"
					type="password"
					class="scroll-hide"
					bind:value
					bind:this={el}
					placeholder={disabled ? "" : placeholder}

Review comment (suggestion):
Unrelated fix but the textbox component was showing a "Type here..." placeholder even when it was disabled. This removes that.

Code after:
			<IconButton
				Icon={copied ? Check : Copy}
				on:click={handle_copy}
				label={copied ? "Copied" : "Copy"}
			/>
		</IconButtonWrapper>
	{/if}
	<BlockTitle show_label={validation_error ? true : show_label} {info}
		>{label}
		{#if validation_error}
			<div class="validation-error">{validation_error}</div>
		{/if}
	</BlockTitle>

	<div class="input-container">
		{#if lines === 1 && _max_lines === 1}
			{#if type === "text"}
				<input
					data-testid="textbox"
					type="text"
					class="scroll-hide"
					dir={rtl ? "rtl" : "ltr"}
					bind:value
					bind:this={el}
					{placeholder}
					{disabled}
					{autofocus}
					maxlength={max_length}
					on:keypress={handle_keypress}
					on:blur
					on:select={handle_select}
					on:focus
					class:validation-error={validation_error}
					style={text_align ? "text-align: " + text_align : ""}
					autocapitalize={html_attributes?.autocapitalize}
					autocorrect={html_attributes?.autocorrect}
					spellcheck={html_attributes?.spellcheck}
					autocomplete={html_attributes?.autocomplete}
					tabindex={html_attributes?.tabindex}
					enterkeyhint={html_attributes?.enterkeyhint}
					lang={html_attributes?.lang}
				/>
			{:else if type === "password"}
				<input
					data-testid="password"
					type="password"
					class="scroll-hide"
					bind:value
					bind:this={el}
					{placeholder}
					{disabled}
Repository: gradio-app/gradio
File: js/core/src/init.svelte.ts (TypeScript)

Code before:
import {
	determine_interactivity,
	get_component,
	get_inputs_outputs
} from "./init_utils";
import { translate_if_needed } from "./i18n";
import { tick, getContext } from "svelte";
import { dequal } from "dequal";

import type {
	ComponentMeta,
	ProcessedComponentMeta,
	LayoutNode,
	Dependency,
	LoadingComponent,
	AppConfig,
	ServerFunctions
} from "./types";
import { type SharedProps, GRADIO_ROOT } from "@gradio/utils";
import { allowed_shared_props } from "@gradio/utils";
import { Client } from "@gradio/client";

type client_return = Awaited<ReturnType<typeof Client.connect>>;

type set_data_type = (data: Record<string, unknown>) => void;
type get_data_type = () => Promise<Record<string, unknown>>;
type visitor<T> = (node: T) => ProcessedComponentMeta;

type Tab = {
	label: string;
	id: string;
	visible: boolean;

Review comment (suggestion):
The main changes are in this file:
- `untrack_children_of_closed_accordions_or_inactive_tabs` sets visibility=False for children of unopened accordions, inactive tabs
- changes to `update_state`. If a component with `visible=False` is the output of an event, we would previously not update its props. Now we do. We also let components with `visible=False`, trigger change events. This was true in 5.x. In order to trigger change events from `update_state`, we have to pass in the `gradio_dispatch_function` from blocks
- Last big change is that tabs/accordions now dispatch custom events that let the AppTree know when to render the children of an Accordion/Tabs. You can see that in `js/tabs/`, `js/accodion`, and `Blocks.svelte` mostly
- 

Code after:
import {
	determine_interactivity,
	get_component,
	get_inputs_outputs
} from "./init_utils";
import { translate_if_needed } from "./i18n";
import { tick } from "svelte";
import { dequal } from "dequal";

import type {
	ComponentMeta,
	ProcessedComponentMeta,
	LayoutNode,
	Dependency,
	LoadingComponent,
	AppConfig,
	ServerFunctions
} from "./types";
import { type SharedProps } from "@gradio/utils";
import { allowed_shared_props } from "@gradio/utils";
import { Client } from "@gradio/client";

type client_return = Awaited<ReturnType<typeof Client.connect>>;

type set_data_type = (data: Record<string, unknown>) => void;
type get_data_type = () => Promise<Record<string, unknown>>;
type visitor<T> = (node: T) => ProcessedComponentMeta;

type Tab = {
	label: string;
	id: string;
	visible: boolean;
Repository: gradio-app/gradio
File: demo/playback_position/run.py (Python)

Code before:
import gradio as gr
from pathlib import Path

# Get the directory where this script is located
demo_dir = Path(__file__).parent

with gr.Blocks() as demo:
    with gr.Tab("Audio"):
        gr.Markdown("## Audio Playback Position")
        gr.Markdown("Click the button to see the current playback position of the audio.")

        audio = gr.Audio(
            value=str(demo_dir / "sax.wav"),
            playback_position=2.0
        )
        audio_btn = gr.Button("Get Audio Playback Position")
        audio_textbox = gr.Textbox(label="Current Playback Position (seconds)")

        def print_audio_playback_pos(a: gr.Audio):
            return f"Audio playback position: {a.playback_position:.2f} seconds"

        audio_btn.click(print_audio_playback_pos, inputs=audio, outputs=audio_textbox)

        set_audio_time_btn = gr.Button("Set Audio Playback Position to 10 seconds")
        def set_audio_playback_pos():
            return gr.Audio(playback_position=10.0)
        
        set_audio_time_btn.click(set_audio_playback_pos, outputs=audio)

    with gr.Tab("Video"):

Review comment (suggestion):
I would add these to `media_data` and use `get_audio`, `get_video`

Code after:
import gradio as gr
from gradio.media import get_audio, get_video

# Get the directory where this script is located
with gr.Blocks() as demo:
    with gr.Tab("Audio"):
        gr.Markdown("## Audio Playback Position")
        gr.Markdown("Click the button to see the current playback position of the audio.")

        audio = gr.Audio(
            value=get_audio("sax.wav"),
            playback_position=2.0,
            elem_id="audio",
        )
        audio_btn = gr.Button("Get Audio Playback Position")
        audio_position = gr.Number(label="Current Audio Position (seconds)")

        def print_audio_playback_pos(a: gr.Audio):
            return a.playback_position

        audio_btn.click(print_audio_playback_pos, inputs=audio, outputs=audio_position)

        set_audio_time_btn = gr.Button("Set Audio Playback Position to 10 seconds")
        def set_audio_playback_pos():
            return gr.Audio(playback_position=10.0)
        
        set_audio_time_btn.click(set_audio_playback_pos, outputs=audio)

    with gr.Tab("Video"):
        gr.Markdown("## Video Playback Position")
Repository: gradio-app/gradio
File: js/dropdown/shared/DropdownOptions.svelte (Svelte)

Code before:
<script lang="ts">
	import { fly } from "svelte/transition";
	let {
		choices,
		filtered_indices,
		show_options = false,
		disabled = false,
		selected_indices = [],
		active_index = null,
		remember_scroll = false,
		offset_from_top = 0,
		from_top = false,
		onchange,
		onload
	}: {
		choices: [string, string | number][];
		filtered_indices: number[];
		show_options: boolean;
		disabled: boolean;
		selected_indices: (string | number)[];
		active_index: number | null;
		remember_scroll?: boolean;
		offset_from_top?: number;
		from_top?: boolean;
		onchange?: (index: any) => void;
		onload?: () => void;
	} = $props();


Review comment (bug):
We need to update Multiselect as well as it uses DropdownOptions. Right now, option selection is broken

![multiselect-bug](https://github.com/user-attachments/assets/fe6e9200-bcd0-4135-ab1c-bdd491284a3b)


Code after:
<script lang="ts">
	import { fly } from "svelte/transition";
	let {
		choices,
		filtered_indices,
		show_options = false,
		disabled = false,
		selected_indices = [],
		active_index = null,
		remember_scroll = false,
		offset_from_top = 0,
		from_top = false,
		onchange,
		onload
	}: {
		choices: [string, string | number][];
		filtered_indices: number[];
		show_options: boolean;
		disabled?: boolean;
		selected_indices?: (string | number)[];
		active_index: number | null;
		remember_scroll?: boolean;
		offset_from_top?: number;
		from_top?: boolean;
		onchange?: (index: any) => void;
		onload?: () => void;
	} = $props();
Repository: gradio-app/gradio
File: js/video/shared/Player.svelte (Svelte)

Code before:
				{/if}
			</span>

			<span class="time">{format_time(time)} / {format_time(duration)}</span>

			<!-- TODO: implement accessible video timeline for 4.0 -->
			<!-- svelte-ignore a11y-click-events-have-key-events -->
			<!-- svelte-ignore a11y-no-noninteractive-element-interactions -->
			<progress
				value={time / duration || 0}
				on:mousemove={handleMove}
				on:touchmove|preventDefault={handleMove}
				on:click|stopPropagation|preventDefault={handle_click}
			/>

			<div class="volume-control-wrapper">
				<button
					class="icon volume-button"
					style:color={show_volume_slider ? "var(--color-accent)" : "white"}
					aria-label="Adjust volume"
					on:click={() => (show_volume_slider = !show_volume_slider)}
				>
					<VolumeLevels {currentVolume} />
				</button>

				{#if show_volume_slider}
					<VolumeControl bind:currentVolume bind:show_volume_slider />
				{/if}
			</div>

			<div
				role="button"
				tabindex="0"
				class="icon"
				aria-label="full-screen"
				on:click={open_full_screen}
				on:keypress={open_full_screen}
			>
				<Maximize />
			</div>
		</div>
	</div>
</div>
{#if interactive}
	<VideoControls
		videoElement={video}
		showRedo
		{handle_trim_video}
		{handle_reset_value}
		bind:processingVideo
		{value}

Review comment (suggestion):
I would hide the maximize icon in `show_volume_slider`. Right now it shrinks whenever the volume controls expand and it looks a bit weird

Code after:
				{/if}
			</span>

			<span class="time">{format_time(time)} / {format_time(duration)}</span>

			<!-- TODO: implement accessible video timeline for 4.0 -->
			<!-- svelte-ignore a11y-click-events-have-key-events -->
			<!-- svelte-ignore a11y-no-noninteractive-element-interactions -->
			<progress
				value={time / duration || 0}
				on:mousemove={handleMove}
				on:touchmove|preventDefault={handleMove}
				on:click|stopPropagation|preventDefault={handle_click}
			/>

			<div class="volume-control-wrapper">
				<button
					class="icon volume-button"
					style:color={show_volume_slider ? "var(--color-accent)" : "white"}
					aria-label="Adjust volume"
					on:click={() => (show_volume_slider = !show_volume_slider)}
				>
					<VolumeLevels currentVolume={current_volume} />
				</button>

				{#if show_volume_slider}
					<VolumeControl bind:current_volume bind:show_volume_slider />
				{/if}
			</div>

			{#if !show_volume_slider}
				<div
					role="button"
					tabindex="0"
					class="icon"
					aria-label="full-screen"
					on:click={open_full_screen}
					on:keypress={open_full_screen}
				>
					<Maximize />
				</div>
			{/if}
		</div>
	</div>
</div>
{#if interactive}
	<VideoControls
		videoElement={video}
		showRedo
		{handle_trim_video}
		{handle_reset_value}
Repository: gradio-app/gradio
File: js/paramviewer/ParamViewer.svelte (Svelte)

Code before:

		return highlighted;
	}

	function highlight_code(
		_docs: typeof docs,
		lang: "python" | "typescript"
	): Param[] {
		if (!_docs) {
			return [];
		}
		return Object.entries(_docs).map(
			([name, { type, description, default: _default }]) => {
				let highlighted_type = type ? highlight(type, lang) : null;

				return {
					name: name,
					type: highlighted_type,
					description: description,
					default: _default ? highlight(_default, lang) : null
				};
			}
		);
	}

	function toggle_all() {
		all_open = !all_open;
		const details = component_root.querySelectorAll(".param");
		details.forEach((detail) => {
			if (detail instanceof HTMLDetailsElement) {
				detail.open = all_open;
			}
		});
	}

	function render_links(description: string): string {
		const escaped = description
			.replace(/&/g, "&amp;")
			.replace(/</g, "&lt;")
			.replace(/>/g, "&gt;")
			.replace(/"/g, "&quot;")
			.replace(/'/g, "&#039;");

		const markdown_links = escaped.replace(
			/\[([^\]]+)\]\(([^)]+)\)/g,
			'<a href="$2" target="_blank">$1</a>'
		);
		return markdown_links;
	}

	onMount(() => {

Review comment (suggestion):
We should keep the returns values typed for clarity.

Code after:

		return highlighted;
	}

	function highlight_code(
		_docs: typeof docs,
		lang: "python" | "typescript"
	): Param[] {
		if (!_docs) {
			return [];
		}
		return Object.entries(_docs).map(
			([name, { type, description, default: _default }]) => {
				let highlighted_type = type ? highlight(type, lang) : null;

				return {
					name: name,
					type: highlighted_type,
					description: description,
					default: _default ? highlight(_default, lang) : null
				};
			}
		);
	}

	function toggle_all(): void {
		all_open = !all_open;
		const details = component_root.querySelectorAll(".param");
		details.forEach((detail) => {
			if (detail instanceof HTMLDetailsElement) {
				detail.open = all_open;
			}
		});
	}

	function render_links(description: string): string {
		const escaped = description
			.replace(/&/g, "&amp;")
			.replace(/</g, "&lt;")
			.replace(/>/g, "&gt;")
			.replace(/"/g, "&quot;")
			.replace(/'/g, "&#039;");

		const markdown_links = escaped.replace(
			/\[([^\]]+)\]\(([^)]+)\)/g,
			'<a href="$2" target="_blank">$1</a>'
		);
		return markdown_links;
	}

	onMount(() => {
Repository: gradio-app/gradio
File: js/multimodaltextbox/shared/MultimodalTextbox.svelte (Svelte)

Code before:
	import { BlockTitle } from "@gradio/atoms";
	import { Upload } from "@gradio/upload";
	import { Image } from "@gradio/image/shared";
	import type { I18nFormatter } from "js/core/src/gradio_helper";
	import type { FileData, Client } from "@gradio/client";
	import type { WaveformOptions } from "@gradio/audio";
	import {
		Clear,
		File,
		Music,
		Paperclip,
		Video,
		ArrowUp,
		Square,
		Microphone,
		Check
	} from "@gradio/icons";
	import type { SelectData } from "@gradio/utils";
	import {
		MinimalAudioPlayer,
		MinimalAudioRecorder
	} from "@gradio/audio/shared";
	import type { InputHTMLAttributes } from "./types";

	let {
		value = $bindable({ text: "", files: [] }),
		value_is_output = $bindable(false),
		lines = 1,
		i18n: _i18n,
		placeholder = "",
		disabled = false,
		label,
		info = undefined,
		show_label = true,
		max_lines,
		submit_btn = null,
		stop_btn = null,
		rtl = false,
		autofocus = false,
		text_align = undefined,
		autoscroll = true,
		root,
		file_types_string = null,
		max_file_size = null,
		upload,
		stream_handler,
		file_count = "multiple",
		max_plain_text_length = 1000,
		waveform_settings,
		waveform_options: _waveform_options = { show_recording_waveform: true },
		sources_string = "upload",

Review comment (suggestion):
You can't bind to a prop that has a fallback value. This is causing the textboxc to not render: https://svelte.dev/docs/svelte/runtime-errors#Client-errors-props_invalid_value

I don't think we need the fallback value though cause the parent Index.svelte already sets the value to be `{ text: "", files: [] }` if it's undefined. Also the parent doesn't bind to the value in the child so I don't think we need the bind either.

Code after:
	import { BlockTitle, ScrollFade } from "@gradio/atoms";
	import { Upload } from "@gradio/upload";
	import { Image } from "@gradio/image/shared";
	import type { I18nFormatter } from "js/core/src/gradio_helper";
	import type { FileData, Client } from "@gradio/client";
	import type { WaveformOptions } from "@gradio/audio";
	import {
		Clear,
		File,
		Music,
		Paperclip,
		Video,
		ArrowUp,
		Square,
		Microphone,
		Check
	} from "@gradio/icons";
	import { should_show_scroll_fade, type SelectData } from "@gradio/utils";
	import {
		MinimalAudioPlayer,
		MinimalAudioRecorder
	} from "@gradio/audio/shared";
	import type { InputHTMLAttributes } from "./types";

	let {
		value = $bindable(),
		value_is_output = false,
		lines = 1,
		i18n: _i18n,
		placeholder = "",
		disabled = false,
		label,
		info = undefined,
		show_label = true,
		max_lines,
		submit_btn = null,
		stop_btn = null,
		rtl = false,
		autofocus = false,
		text_align = undefined,
		autoscroll = true,
		root,
		file_types_string = null,
		max_file_size = null,
		upload,
		stream_handler,
		file_count = "multiple",
		max_plain_text_length = 1000,
		waveform_settings,
		waveform_options: _waveform_options = { show_recording_waveform: true },
		sources_string = "upload",