ronantakizawa/codereview-bench huggingface dataset:
Repository: gradio-app/gradio
File: js/multimodaltextbox/shared/MultimodalTextbox.svelte (Svelte)

Code before:
			resize(el, lines, max_lines);
		}
	});

	$effect.pre(() => {
		if (el && el.offsetHeight + el.scrollTop > el.scrollHeight - 100) {
			can_scroll = true;
		}
	});

	const scroll = (): void => {
		if (can_scroll && autoscroll && !user_has_scrolled_up) {
			el.scrollTo(0, el.scrollHeight);
		}
	};

	async function handle_change(): Promise<void> {
		onchange?.(value);
		if (!value_is_output) {
			oninput?.();
		}
	}

	$effect(() => {
		if (autofocus && el) {
			el.focus();
		}
	});

	$effect(() => {
		if (can_scroll && autoscroll) {
			scroll();
		}
	});

	function handle_select(event: Event): void {
		const target: HTMLTextAreaElement | HTMLInputElement = event.target as
			| HTMLTextAreaElement
			| HTMLInputElement;
		const text = target.value;
		const index: [number, number] = [
			target.selectionStart as number,
			target.selectionEnd as number
		];
		onselect?.({ value: text.substring(...index), index: index });
	}

	async function handle_keypress(e: KeyboardEvent): Promise<void> {
		if (e.key === "Enter" && e.shiftKey && lines > 1) {
			e.preventDefault();
			await tick();

Review comment (bug):
I think we need to access value here so that this triggers whenever the value changes. That was the behavior before (using afterUpdate) but that got deprecated. Right now this effect only runs once and it's why a playwright test is failing

Code after:
			resize(el, lines, max_lines);
		}
	});

	$effect.pre(() => {
		if (el && el.offsetHeight + el.scrollTop > el.scrollHeight - 100) {
			can_scroll = true;
		}
	});

	const scroll = (): void => {
		if (can_scroll && autoscroll && !user_has_scrolled_up) {
			el.scrollTo(0, el.scrollHeight);
		}
	};

	async function handle_change(): Promise<void> {
		onchange?.(value);
		if (!value_is_output) {
			oninput?.();
		}
	}

	$effect(() => {
		value;
		if (autofocus && el) {
			el.focus();
		}
	});

	$effect(() => {
		if (can_scroll && autoscroll) {
			scroll();
		}
	});

	function handle_select(event: Event): void {
		const target: HTMLTextAreaElement | HTMLInputElement = event.target as
			| HTMLTextAreaElement
			| HTMLInputElement;
		const text = target.value;
		const index: [number, number] = [
			target.selectionStart as number,
			target.selectionEnd as number
		];
		onselect?.({ value: text.substring(...index), index: index });
	}

	async function handle_keypress(e: KeyboardEvent): Promise<void> {
		if (e.key === "Enter" && e.shiftKey && lines > 1) {
			e.preventDefault();
Repository: gradio-app/gradio
File: js/gallery/shared/utils.ts (TypeScript)

Code before:
import { FileData } from "@gradio/client";

export async function format_gallery_for_sharing(
	value: [FileData, string | null][] | null
): Promise<string> {
	if (!value) return "";
	let urls = await Promise.all(
		value.map(async ([image, _]) => {
			if (image === null || !image.url) return "";
			return await uploadToHuggingFace(image.url, "url");
		})
	);

	return `<div style="display: flex; flex-wrap: wrap; gap: 16px">${urls
		.map((url) => `<img src="${url}" style="height: 400px" />`)
		.join("")}</div>`;
}

export async function handle_save(
	img_blob: Blob | any,
	upload: any
): Promise<FileData[]> {
	const f_ = new File([img_blob], `image.png`);
	const files = [
		new FileData({
			path: f_.name,
			orig_name: f_.name,
			blob: f_,
			size: f_.size,
			mime_type: f_.type,
			is_stream: false
		})
	];
	return await upload(...files);
}


Review comment (suggestion):
i think we should use the filename and the mime type when saving instead of hardcoding to image.png

```suggestion
export async function handle_save(
	img_blob: Blob | any,
	upload: any,
    filename = "file" // or maybe theres sth better than file
): Promise<FileData[]> {
    const ext = blob.type.split("/")[1] || "png";                     
	const f_ = new File([img_blob], `${filename}.${ext}`);
```

Code after:
import { FileData, Client } from "@gradio/client";

export async function format_gallery_for_sharing(
	value: [FileData, string | null][] | null
): Promise<string> {
	if (!value) return "";
	let urls = await Promise.all(
		value.map(async ([image, _]) => {
			if (image === null || !image.url) return "";
			return await uploadToHuggingFace(image.url, "url");
		})
	);

	return `<div style="display: flex; flex-wrap: wrap; gap: 16px">${urls
		.map((url) => `<img src="${url}" style="height: 400px" />`)
		.join("")}</div>`;
}

export async function handle_save(
	img_blob: Blob,
	upload: Client["upload"],
	filename: string = "uploaded_file"
): Promise<FileData[]> {
	const ext = img_blob.type.split("/")[1] || "png";
	const f_ = new File([img_blob], `${filename}.${ext}`);
	const files = [
		new FileData({
			path: f_.name,
			orig_name: f_.name,
			blob: f_,
			size: f_.size,
			mime_type: f_.type,
			is_stream: false
		})
	];
	return (await upload(...files)) as FileData[];
}
Repository: gradio-app/gradio
File: js/fileexplorer/Index.svelte (Svelte)

Code before:
<script lang="ts">
	import type { FileExplorerProps, FileExplorerEvents } from "./types";
	import { Gradio } from "@gradio/utils";
	import { File } from "@gradio/icons";

	import { Block, BlockLabel, IconButtonWrapper } from "@gradio/atoms";
	import DirectoryExplorer from "./shared/DirectoryExplorer.svelte";

	import { StatusTracker } from "@gradio/statustracker";

	import { _ } from "svelte-i18n";

	const props = $props();
	const gradio = new Gradio<FileExplorerEvents, FileExplorerProps>(props);

	gradio.props.value = gradio.props.value ?? [];
	let old_value = $state(gradio.props.value);

	let rerender_key = $derived([
		gradio.props.root_dir,
		gradio.props.glob,
		gradio.props.ignore_glob
	]);

	// Reset value when rerender_key changes
	// svelte-ignore state_referenced_locally
	let old_rerender_key = $state(rerender_key);
	$effect(() => {
		if (JSON.stringify(old_rerender_key) != JSON.stringify(rerender_key) && old_value == gradio.props.value) {
			old_rerender_key = rerender_key;
			gradio.props.value = [];
		}
	});

	$effect(() => {
		if (old_value != gradio.props.value) {
			old_value = gradio.props.value;
			gradio.dispatch("change");
		}
	});
</script>

Review comment (suggestion):
You can set default values in the `Gradio` class constructor. 

```js
const gradio = new Gradio<FileExplorerEvents, FileExplorerProps>(props, {value: []});
```

Code after:
<script lang="ts">
	import type { FileExplorerProps, FileExplorerEvents } from "./types";
	import { Gradio } from "@gradio/utils";
	import { File } from "@gradio/icons";

	import { Block, BlockLabel, IconButtonWrapper } from "@gradio/atoms";
	import DirectoryExplorer from "./shared/DirectoryExplorer.svelte";

	import { StatusTracker } from "@gradio/statustracker";

	import { _ } from "svelte-i18n";

	const props = $props();
	const gradio = new Gradio<FileExplorerEvents, FileExplorerProps>(props, {
		value: []
	});

	let old_value = $state(gradio.props.value);

	let rerender_key = $derived([
		gradio.props.root_dir,
		gradio.props.glob,
		gradio.props.ignore_glob
	]);

	// Reset value when rerender_key changes
	// svelte-ignore state_referenced_locally
	let old_rerender_key = $state(rerender_key);
	$effect(() => {
		if (
			JSON.stringify(old_rerender_key) != JSON.stringify(rerender_key) &&
			old_value == gradio.props.value
		) {
			old_rerender_key = rerender_key;
			gradio.props.value = [];
		}
	});

	$effect(() => {
		if (old_value != gradio.props.value) {
			old_value = gradio.props.value;
Repository: gradio-app/gradio
File: js/dataset/Dataset.svelte (Svelte)

Code before:
		proxy_url ? `/proxy=${proxy_url}file=` : `${root}/file=`
	);

	let page = $state(0);
	let current_hover = $state(-1);
	let old_samples_json = $state("");

	let gallery = $derived(
		(components.length < 2 || sample_labels !== null) && layout !== "table"
	);

	let effective_samples = $derived.by(() => {
		if (sample_labels) {
			return sample_labels.map((e) => [e]);
		}
		return samples ?? [];
	});

	// Reset page when samples change
	$effect(() => {
		const current_json = JSON.stringify(effective_samples);
		if (current_json !== old_samples_json) {
			page = 0;
			old_samples_json = current_json;
		}
	});

	let paginate = $derived(effective_samples.length > samples_per_page);

	let selected_samples = $derived.by(() => {
		if (paginate) {
			return effective_samples.slice(
				page * samples_per_page,
				(page + 1) * samples_per_page
			);
		}
		return effective_samples.slice();
	});

	let page_count = $derived(
		Math.ceil(effective_samples.length / samples_per_page)
	);

	let visible_pages = $derived.by(() => {
		if (!paginate) return [];
		let pages: number[] = [];
		[0, page, page_count - 1].forEach((anchor) => {
			for (let i = anchor - 2; i <= anchor + 2; i++) {
				if (i >= 0 && i < page_count && !pages.includes(i)) {
					if (pages.length > 0 && i - pages[pages.length - 1] > 1) {
						pages.push(-1);

Review comment (bug):
An effect isn't the correct choice here. We should avoid updating state inside an effect wherever possible as it is a common source of bugs. 

We could try making `page` and `old_samples_json` derived values or just update them directly in an event handler.

Code after:
		proxy_url ? `/proxy=${proxy_url}file=` : `${root}/file=`
	);

	let current_hover = $state(-1);

	let gallery = $derived(
		(components.length < 2 || sample_labels !== null) && layout !== "table"
	);

	let effective_samples = $derived.by(() => {
		if (sample_labels) {
			return sample_labels.map((e) => [e]);
		}
		return samples ?? [];
	});

	// page resets to 0 whenever effective_samples changes,
	// but can still be overwritten by user clicks
	let page = $derived.by(() => {
		effective_samples;
		return 0;
	});

	let paginate = $derived(effective_samples.length > samples_per_page);

	let selected_samples = $derived.by(() => {
		if (paginate) {
			return effective_samples.slice(
				page * samples_per_page,
				(page + 1) * samples_per_page
			);
		}
		return effective_samples.slice();
	});

	let page_count = $derived(
		Math.ceil(effective_samples.length / samples_per_page)
	);

	let visible_pages = $derived.by(() => {
		if (!paginate) return [];
		let pages: number[] = [];
		[0, page, page_count - 1].forEach((anchor) => {
			for (let i = anchor - 2; i <= anchor + 2; i++) {
				if (i >= 0 && i < page_count && !pages.includes(i)) {
					if (pages.length > 0 && i - pages[pages.length - 1] > 1) {
						pages.push(-1);
					}
					pages.push(i);
				}
			}
Repository: gradio-app/gradio
File: js/colorpicker/shared/Colorpicker.svelte (Svelte)

Code before:
			style="--hue:{hue}"
			bind:this={sl_wrap}
		>
			<div
				class="marker"
				style:transform="translate({sl_marker_pos[0]}px,{sl_marker_pos[1]}px)"
				style:background={value}
			/>
		</div>
		<!-- svelte-ignore a11y-no-static-element-interactions -->
		<div class="hue-slider" on:mousedown={handle_hue_down} bind:this={hue_wrap}>
			<div
				class="marker"
				style:background={"hsl(" + hue + ", 100%, 50%)"}
				style:transform="translateX({hue_marker_pos}px)"
			/>
		</div>

		<div class="input">
			<button class="swatch" style:background={value}></button>
			<div>
				<div class="input-wrap">
					<input
						type="text"
						bind:value={color_string}
						on:change={(e) => {
							value = e.currentTarget.value;
						}}
						onkeydown={(e) => {
							if (e.key === "Enter") {
								on_submit();
							}
						}}
					/>
					<button class="eyedropper" on:click={request_eyedropper}>
						{#if eyedropper_supported}
							<Eyedropper />
						{/if}
					</button>
				</div>

				<div class="buttons">
					{#each modes as [label, value]}
						<button
							class="button"
							class:active={current_mode === value}
							on:click={() => {
								current_mode = value;
							}}>{label}</button
						>
					{/each}

Review comment (suggestion):
This looks good! But just a note that svelte 5 does not let you mix `on:change` (with a colon) and `onfocus` (no colon). Please change them to be without a colon!

Code after:
			<div
				class="marker"
				style:transform="translate({sl_marker_pos[0]}px,{sl_marker_pos[1]}px)"
				style:background={value}
			/>
		</div>
		<!-- svelte-ignore a11y-no-static-element-interactions -->
		<div class="hue-slider" onmousedown={handle_hue_down} bind:this={hue_wrap}>
			<div
				class="marker"
				style:background={"hsl(" + hue + ", 100%, 50%)"}
				style:transform="translateX({hue_marker_pos}px)"
			/>
		</div>

		<div class="input">
			<button class="swatch" style:background={value}></button>
			<div>
				<div class="input-wrap">
					<input
						type="text"
						bind:value={color_string}
						onchange={(e) => {
							value = e.currentTarget.value;
						}}
						onkeydown={(e) => {
							if (e.key === "Enter") {
								on_submit();
							}
						}}
					/>
					<button class="eyedropper" onclick={request_eyedropper}>
						{#if eyedropper_supported}
							<Eyedropper />
						{/if}
					</button>
				</div>

				<div class="buttons">
					{#each modes as [label, value]}
						<button
							class="button"
							class:active={current_mode === value}
							onclick={() => {
								current_mode = value;
							}}>{label}</button
						>
					{/each}
				</div>
			</div>
		</div>
Repository: gradio-app/gradio
File: js/core/src/dependency.ts (TypeScript)

Code before:
	async cancel(ids: number[] | undefined): Promise<void> {
		if (!ids) return;

		for (const id of ids) {
			const submission = this.submissions.get(id);
			if (submission) {
				await submission.cancel();
				this.loading_stati.update({
					status: "complete",
					fn_index: id,
					eta: 0,
					queue: false,
					stream_state: null
				});
				this.update_loading_stati_state();
				this.submissions.delete(id);
				// Need to trigger any dependencies that are waiting for this one to complete
				const { failure, all } = this.dependencies_by_fn
					.get(id)
					?.get_triggers() || { failure: [], all: [] };
				failure.forEach((dep_id) => {
					this.dispatch({
						type: "fn",
						fn_index: dep_id,
						event_data: null,
						target_id: id as number | undefined
					});
				});
				all.forEach((dep_id) => {
					this.dispatch({
						type: "fn",
						fn_index: dep_id,
						event_data: null,
						target_id: id as number | undefined
					});
				});
			}
		}
	}

	dispatch_load_events() {
		this.dependencies_by_fn.forEach((dep) => {
			dep.targets.forEach(([target_id, event_name]) => {
				if (event_name === "load") {
					this.dispatch({
						type: "fn",
						fn_index: dep.id,
						event_data: null,
						target_id: target_id
					});
				}

Review comment (suggestion):
i don't think `as number | undefined` casting is needed here as it's already typed as a number 

Code after:
	async cancel(ids: number[] | undefined): Promise<void> {
		if (!ids) return;

		for (const id of ids) {
			const submission = this.submissions.get(id);
			if (submission) {
				await submission.cancel();
				this.loading_stati.update({
					status: "complete",
					fn_index: id,
					eta: 0,
					queue: false,
					stream_state: null
				});
				this.update_loading_stati_state();
				this.submissions.delete(id);
				// Need to trigger any dependencies that are waiting for this one to complete
				const { failure, all } = this.dependencies_by_fn
					.get(id)
					?.get_triggers() || { failure: [], all: [] };
				failure.forEach((dep_id) => {
					this.dispatch({
						type: "fn",
						fn_index: dep_id,
						event_data: null,
						target_id: id
					});
				});
				all.forEach((dep_id) => {
					this.dispatch({
						type: "fn",
						fn_index: dep_id,
						event_data: null,
						target_id: id
					});
				});
			}
		}
	}

	dispatch_load_events() {
		this.dependencies_by_fn.forEach((dep) => {
			dep.targets.forEach(([target_id, event_name]) => {
				if (event_name === "load") {
					this.dispatch({
						type: "fn",
						fn_index: dep.id,
						event_data: null,
						target_id: target_id
					});
				}
Repository: gradio-app/gradio
File: js/html/shared/HTML.svelte (Svelte)

Code before:
							property === "visible"
						) {
							props[property] = value;
							old_props[property] = value;
							dispatch("update_value", { data: value, property });
						}
					}
					return true;
				}
			}
		);

		if (has_children) {
			currentPreHtml = render_template(
				pre_html_template,
				reactiveProps,
				"html"
			);
			pre_element.innerHTML = currentPreHtml;
			currentPostHtml = render_template(
				post_html_template,
				reactiveProps,
				"html"
			);
			post_element.innerHTML = currentPostHtml;
		} else {
			currentHtml = render_template(html_template, reactiveProps, "html");
			element.innerHTML = currentHtml;
		}
		update_css();

		if (autoscroll) {
			scroll_to_bottom();
		}
		scroll_on_html_update();

		if (js_on_load && element) {
			try {
				const func = new Function("element", "trigger", "props", js_on_load);
				func(element, trigger, reactiveProps);
			} catch (error) {
				console.error("Error executing js_on_load:", error);
			}
		}
	});

	// Props update effect
	$effect(() => {
		if (
			reactiveProps &&
			props &&

Review comment (bug):
I think something is wrong with the css scoping. If you place an Image in side the layout, the image looks off (buttons rounded, background color changes on hover). Might be the `prose` css class?

<img width="1261" height="534" alt="Image" src="https://github.com/user-attachments/assets/e962abb0-2181-45e8-9692-891f546bd23a" />

Code after:
		reactiveProps = new Proxy(
			{ ...props },
			{
				set(target, property, value) {
					const oldValue = target[property as string];
					target[property as string] = value;

					if (oldValue !== value) {
						scheduleRender();

						if (
							property === "value" ||
							property === "label" ||
							property === "visible"
						) {
							props[property] = value;
							old_props[property] = value;
							dispatch("update_value", { data: value, property });
						}
					}
					return true;
				}
			}
		);

		if (has_children) {
			currentPreHtml = render_template(
				pre_html_template,
				reactiveProps,
				"html"
			);
			pre_element.innerHTML = currentPreHtml;
			currentPostHtml = render_template(
				post_html_template,
				reactiveProps,
				"html"
			);
			post_element.innerHTML = currentPostHtml;
		} else {
			currentHtml = render_template(html_template, reactiveProps, "html");
			element.innerHTML = currentHtml;
		}
		update_css();

		if (autoscroll) {
			scroll_to_bottom();
		}
		scroll_on_html_update();

		if (js_on_load && element) {
			try {
Repository: gradio-app/gradio
File: js/core/src/init.svelte.ts (TypeScript)

Code before:
			(node) => handle_visibility(node, this.#config.api_url)
		]);
	}
}

function make_visible_if_not_rendered(
	node: ProcessedComponentMeta,
	hidden_on_startup: Set<number>,
	isTargetNode = false
): void {
	node.props.shared_props.visible = hidden_on_startup.has(node.id)
		? true
		: node.props.shared_props.visible;

	if (node.type === "tabs") {
		const selectedId =
			node.props.props.selected ?? node.props.props.initial_tabs?.[0]?.id;
		node.children.forEach((child) => {
			if (
				child.type === "tabitem" &&
				(child.props.props.id === selectedId || child.id === selectedId)
			) {
				make_visible_if_not_rendered(child, hidden_on_startup, false);
			}
		});
	} else if (
		node.type === "accordion" &&
		node.props.props.open === false &&
		!isTargetNode
	) {
	} else if (node.type === "accordion") {
		node.children.forEach((child) => {
			make_visible_if_not_rendered(child, hidden_on_startup, false);
		});
	} else {
		node.children.forEach((child) => {
			make_visible_if_not_rendered(child, hidden_on_startup, false);
		});
	}
}

/**
 * Process the server function names and return a dictionary of functions
 * @param id the component id
 * @param server_fns the server function names
 * @param app the client instance
 * @returns the actual server functions
 */
export function process_server_fn(
	id: number,
	server_fns: string[] | undefined,

Review comment (suggestion):
I think we can rewrite to make this clearer:
* remove the empty case if accordion close
* I think we can merge the open accordion and "else" case since they are identical

Code after:
			(node) => handle_visibility(node, this.#config.api_url)
		]);
	}
}

function make_visible_if_not_rendered(
	node: ProcessedComponentMeta,
	hidden_on_startup: Set<number>,
	is_target_node = false
): void {
	node.props.shared_props.visible = hidden_on_startup.has(node.id)
		? true
		: node.props.shared_props.visible;

	if (node.type === "tabs") {
		const selectedId =
			node.props.props.selected ?? node.props.props.initial_tabs?.[0]?.id;
		node.children.forEach((child) => {
			if (
				child.type === "tabitem" &&
				(child.props.props.id === selectedId || child.id === selectedId)
			) {
				make_visible_if_not_rendered(child, hidden_on_startup, false);
			}
		});
	} else if (
		node.type === "accordion" &&
		node.props.props.open === false &&
		!is_target_node
	) {
		// Don't recurse into closed accordion content
	} else {
		node.children.forEach((child) => {
			make_visible_if_not_rendered(child, hidden_on_startup, false);
		});
	}
}

/**
 * Process the server function names and return a dictionary of functions
 * @param id the component id
 * @param server_fns the server function names
 * @param app the client instance
 * @returns the actual server functions
 */
export function process_server_fn(
	id: number,
	server_fns: string[] | undefined,
	app: client_return
): ServerFunctions {
	if (!server_fns) {
Repository: gradio-app/gradio
File: js/core/src/Blocks.svelte (Svelte)

Code before:
		if (!api_recorder_visible) return;
		api_calls = [...api_calls, last_api_call];
	};

	function handle_connection_lost(): void {
		if (connection_lost) return;
		connection_lost = true;

		messages = messages.filter((m) => m.type !== "error");

		++_error_id;
		messages.push({
			title: "Connection Lost",
			message: LOST_CONNECTION_MESSAGE,
			fn_index: -1,
			type: "error",
			id: _error_id,
			duration: null,
			visible: true
		});

		reconnect_interval = setInterval(async () => {
			try {
				const status = await app.reconnect();
				if (status === "connected" || status === "changed") {
					clearInterval(reconnect_interval!);
					reconnect_interval = null;
					window.location.reload();
				}
			} catch (e) {
				// server still unreachable
			}
		}, 2000);
	}

	let dep_manager = new DependencyManager(
		dependencies,
		app,
		app_tree.update_state.bind(app_tree),
		app_tree.get_state.bind(app_tree),
		app_tree.rerender.bind(app_tree),
		new_message,
		add_to_api_calls,
		handle_connection_lost
	);

	$effect(() => {
		reload_count;
		untrack(() => {
			app_tree.reload(components, layout, dependencies, {
				root,

Review comment (bug):
Should probably still do a console.debug in the catch though in case the exception is caused by something other than the server being down.

Code after:
		api_calls = [...api_calls, last_api_call];
	};

	function handle_connection_lost(): void {
		messages = messages.filter((m) => m.type !== "error");

		++_error_id;
		messages.push({
			title: "Connection Lost",
			message: LOST_CONNECTION_MESSAGE,
			fn_index: -1,
			type: "error",
			id: _error_id,
			duration: null,
			visible: true
		});

		reconnect_interval = setInterval(async () => {
			try {
				const status = await app.reconnect();
				if (status === "connected" || status === "changed") {
					clearInterval(reconnect_interval!);
					reconnect_interval = null;
					window.location.reload();
				}
			} catch (e) {
				// server still unreachable
				console.debug(e);
			}
		}, 2000);
	}

	let dep_manager = new DependencyManager(
		dependencies,
		app,
		app_tree.update_state.bind(app_tree),
		app_tree.get_state.bind(app_tree),
		app_tree.rerender.bind(app_tree),
		new_message,
		add_to_api_calls,
		handle_connection_lost
	);

	$effect(() => {
		reload_count;
		untrack(() => {
			app_tree.reload(components, layout, dependencies, {
				root,
				theme: theme_mode,
				version,
				api_prefix,