ronantakizawa/codereview-bench huggingface dataset:
Repository: gradio-app/gradio
File: gradio/components/number.py (Python)

Code before:
        Returns:
            The (optionally rounded) field value as a `float` or `int` depending on `precision`.
        """
        if value is None:
            return None
        return self.round_to_precision(value, self.precision)

    def api_info(self) -> dict[str, str]:
        if self.precision == 0:
            return {"type": "integer"}
        return {"type": "number"}

    def example_payload(self) -> Any:
        return self.round_to_precision(
            3 if self.minimum is None else self.minimum, self.precision
        )

    def example_value(self) -> Any:
        return self.round_to_precision(
            3 if self.minimum is None else self.minimum, self.precision
        )

    def read_from_flag(self, payload: str):
        import ast

        return ast.literal_eval(payload)


Review comment (suggestion):
would be safer to do `float()` no?
```suggestion
        return float(payload)
```

Code after:
            value: Expects an `int` or `float` returned from the function and sets field value to it.
        Returns:
            The (optionally rounded) field value as a `float` or `int` depending on `precision`.
        """
        if value is None:
            return None
        return self.round_to_precision(value, self.precision)

    def api_info(self) -> dict[str, str]:
        if self.precision == 0:
            return {"type": "integer"}
        return {"type": "number"}

    def example_payload(self) -> Any:
        return self.round_to_precision(
            3 if self.minimum is None else self.minimum, self.precision
        )

    def example_value(self) -> Any:
        return self.round_to_precision(
            3 if self.minimum is None else self.minimum, self.precision
        )

    def read_from_flag(self, payload: str):
        """Numbers are stored as strings in the flagging file, so we need to parse them as json."""
        return json.loads(payload)
Repository: gradio-app/gradio
File: gradio/components/browser_state.py (Python)

Code before:
            Passes value through unchanged
        """
        if payload is None:
            return self.default_value
        return payload

    def postprocess(self, value: Any) -> Any:
        """
        Parameters:
            value: Value to store in local storage
        Returns:
            Passes value through unchanged
        """
        return value

    def api_info(self) -> dict[str, Any]:
        return {"type": {}, "description": "any json-serializable value"}

    def example_payload(self) -> Any:
        return "test"

    def example_value(self) -> Any:
        return "test"

    def breaks_grouping(self) -> bool:
        """State components should not break wrapper grouping chains."""
        return False


Review comment (suggestion):
```suggestion
        """BrowserState components should not break wrapper grouping chains."""
```

Code after:
            Passes value through unchanged
        """
        if payload is None:
            return self.default_value
        return payload

    def postprocess(self, value: Any) -> Any:
        """
        Parameters:
            value: Value to store in local storage
        Returns:
            Passes value through unchanged
        """
        return value

    def api_info(self) -> dict[str, Any]:
        return {"type": {}, "description": "any json-serializable value"}

    def example_payload(self) -> Any:
        return "test"

    def example_value(self) -> Any:
        return "test"

    def breaks_grouping(self) -> bool:
        """BrowserState components should not break wrapper grouping chains."""
        return False
Repository: gradio-app/gradio
File: js/core/src/api_docs/ApiDocs.svelte (Svelte)

Code before:
		const signal = controller.signal;

		document.body.style.overflow = "hidden";
		if ("parentIFrame" in window) {
			window.parentIFrame?.scrollTo(0, 0);
		}

		const lang_param = get_query_param("lang");
		if (is_valid_language(lang_param)) {
			current_language = lang_param as "python" | "javascript" | "bash" | "mcp";
		}

		// Check MCP server status and fetch tools if active
		fetch(mcp_server_url, { signal: signal })
			.then((response) => {
				mcp_server_active = response.ok;
				if (mcp_server_active) {
					fetch_mcp_tools();
					if (!is_valid_language(lang_param)) {
						current_language = "mcp";
					}
				} else {
					if (!is_valid_language(lang_param)) {
						current_language = "python";
					}
				}
				controller.abort();
			})
			.catch(() => {
				mcp_server_active = false;
			});

		return () => {
			document.body.style.overflow = "auto";
		};
	});
</script>

{#if info && analytics}
	{#if api_count}
		<div class="banner-wrap">
			<ApiBanner
				on:close
				root={space_id || root}
				{api_count}
				{current_language}
			/>
		</div>

		<div class="docs-wrap">
			<div class="client-doc">

Review comment (suggestion):
Fetching the mcp_server_url opens a persistent connection. That means opening the view api page 5 or more times from the same browser session causes the whole page to freeze because we reach the 5 concurrent connection limit. So I changed it to close the connection immediately. 

Code after:

		document.body.style.overflow = "hidden";
		if ("parentIFrame" in window) {
			window.parentIFrame?.scrollTo(0, 0);
		}

		const lang_param = get_query_param("lang");
		if (is_valid_language(lang_param)) {
			current_language = lang_param as "python" | "javascript" | "bash" | "mcp";
		}

		// Check MCP server status and fetch tools if active
		fetch(mcp_server_url, { signal: signal })
			.then((response) => {
				mcp_server_active = response.ok;
				if (mcp_server_active) {
					fetch_mcp_tools();
					if (!is_valid_language(lang_param)) {
						current_language = "mcp";
					}
				} else {
					if (!is_valid_language(lang_param)) {
						current_language = "python";
					}
				}
				controller.abort();
			})
			.catch(() => {
				mcp_server_active = false;
			});

		return () => {
			document.body.style.overflow = "auto";
		};
	});
</script>

{#if info && analytics}
	{#if api_count}
		<div class="banner-wrap">
			<ApiBanner
				on:close
				root={space_id || root}
				{api_count}
				{current_language}
			/>
		</div>

		<div class="docs-wrap">
			<div class="client-doc">
				<p style="font-size: var(--text-lg);">
Repository: gradio-app/gradio
File: js/gallery/shared/Gallery.svelte (Svelte)

Code before:
		max-width: 80%;
		overflow: hidden;
		font-size: var(--block-label-text-size);
		text-align: left;
		text-overflow: ellipsis;
		white-space: nowrap;
	}

	.grid-wrap.minimal {
		padding: 0;
	}

	.gallery-item {
		position: relative;
		width: 100%;
		height: 100%;
	}

	.delete-button {
		position: absolute;
		bottom: 0;
		left: 0;
		z-index: var(--layer-1);
		border-top: 1px solid var(--border-color-primary);
		border-right: 1px solid var(--border-color-primary);
		border-radius: 0 var(--radius-sm) 0 0;
		background: var(--background-fill-secondary);
		padding: var(--block-label-padding);
		cursor: pointer;
		display: flex;
		align-items: center;
		justify-content: center;
		opacity: 0;
		transition: opacity 0.2s ease;
		font-size: var(--block-label-text-size);
		color: var(--block-label-text-color);
		font-weight: var(--weight-semibold);
		width: auto;
		height: auto;
		min-width: fit-content;
		min-height: fit-content;
	}

	.gallery-item:hover .delete-button {
		opacity: 1;
	}

	.delete-button:hover {
		opacity: 0.8;
	}


Review comment (suggestion):
```suggestion
		border-radius: 0 var(--radius-sm) 0 var(--radius-sm);
```

Code after:
		max-width: 80%;
		overflow: hidden;
		font-size: var(--block-label-text-size);
		text-align: left;
		text-overflow: ellipsis;
		white-space: nowrap;
	}

	.grid-wrap.minimal {
		padding: 0;
	}

	.gallery-item {
		position: relative;
		width: 100%;
		height: 100%;
	}

	.delete-button {
		position: absolute;
		bottom: 0;
		left: 0;
		z-index: var(--layer-1);
		border-top: 1px solid var(--border-color-primary);
		border-right: 1px solid var(--border-color-primary);
		border-radius: 0 var(--radius-sm) 0 var(--radius-sm);
		background: var(--background-fill-secondary);
		padding: var(--block-label-padding);
		cursor: pointer;
		display: flex;
		align-items: center;
		justify-content: center;
		opacity: 0;
		transition: opacity 0.2s ease;
		font-size: var(--block-label-text-size);
		color: var(--block-label-text-color);
		font-weight: var(--weight-semibold);
		width: auto;
		height: auto;
		min-width: fit-content;
		min-height: fit-content;
	}

	.gallery-item:hover .delete-button {
		opacity: 1;
	}

	.delete-button:hover {
		opacity: 0.8;
	}
Repository: gradio-app/gradio
File: js/core/src/init.ts (TypeScript)

Code before:
				node.children.map((v) => walk_layout(v, root, components, instance))
			);
		}

		if (instance.type === "tabs" && !instance.props.initial_tabs) {
			const tab_items_props =
				node.children?.map((c, i) => {
					const instance = instance_map[c.id];
					instance.props.id ??= c.id;
					return {
						type: instance.type,
						props: {
							...(instance.props as any),
							id: instance.props.id,
							order: i
						}
					};
				}) || [];

			const child_tab_items = tab_items_props.filter(
				(child) => child.type === "tabitem"
			);

			instance.props.initial_tabs = child_tab_items?.map((child) => ({
				label: child.props.label,
				id: child.props.id,
				visible:
					typeof child.props.visible === "boolean" ? child.props.visible : true,
				interactive: child.props.interactive,
				order: child.props.order
			}));
		}

		if (instance.type === "tabs") {
			node.children?.forEach((c, i) => {
				const child = instance_map[c.id];
				child.props.order = i;
			});
		}

		return instance;
	}

	let update_scheduled = false;
	let update_scheduled_store = writable(false);

	/**
	 * Load newly visible components after visibility changes
	 * @param newly_visible_ids Set of component IDs that are now visible
	 */
	async function load_newly_visible_components(

Review comment (suggestion):
Made this change so that the entire i18n dict does not show up but tbh it is on the page so briefly I think we can also not do this

Code after:
			instance.children = await Promise.all(
				node.children.map((v) => walk_layout(v, root, components, instance))
			);
		}

		if (instance.type === "tabs" && !instance.props.initial_tabs) {
			const tab_items_props =
				node.children?.map((c, i) => {
					const instance = instance_map[c.id];
					instance.props.id ??= c.id;
					return {
						type: instance.type,
						props: {
							...(instance.props as any),
							id: instance.props.id,
							order: i
						}
					};
				}) || [];

			const child_tab_items = tab_items_props.filter(
				(child) => child.type === "tabitem"
			);

			instance.props.initial_tabs = child_tab_items?.map((child) => ({
				label: child.props.label.includes(i18n_marker) ? "" : child.props.label,
				id: child.props.id,
				visible:
					typeof child.props.visible === "boolean" ? child.props.visible : true,
				interactive: child.props.interactive,
				order: child.props.order
			}));
		}

		if (instance.type === "tabs") {
			node.children?.forEach((c, i) => {
				const child = instance_map[c.id];
				child.props.order = i;
			});
		}

		return instance;
	}

	let update_scheduled = false;
	let update_scheduled_store = writable(false);

	/**
	 * Load newly visible components after visibility changes
	 * @param newly_visible_ids Set of component IDs that are now visible
	 */
Repository: gradio-app/gradio
File: js/navbar/Navbar.stories.svelte (Svelte)

Code before:
<script>
	import { Meta, Template, Story } from "@storybook/addon-svelte-csf";
	import { Embed } from "@gradio/core";

	/** @type {HTMLDivElement} */
	let wrapper;
	const pages = /** @type {[string, string, boolean][]} */ ([
		["", "Home", true],
		["page1", "Page 1", true],
		["page2", "Page 2", true],
		["settings", "Settings", true]
	]);
</script>

<Meta
	title="Components/Navbar"
	argTypes={{
		visible: {
			options: [true, false],
			description: "Sets the visibility of the navbar",
			control: { type: "boolean" },
			defaultValue: true
		},
		main_page_name: {
			control: "text",
			description:
				"The name to display for the main page in the navbar. Set to false to use default 'Home'",
			name: "main_page_name",
			value: "Home"
		},
		value: {
			control: "object",
			description:
				"List of [route, name] tuples for additional navbar pages. These are added to the existing pages",
			name: "value",

Review comment (suggestion):
```suggestion
		["page2", "Page 2", false],
```

if we wanted a quick test for this we should add a page with no visibility so it'll flag if the vis logic ever breaks

Code after:
<script>
	import { Meta, Template, Story } from "@storybook/addon-svelte-csf";
	import { Embed } from "@gradio/core";

	/** @type {HTMLDivElement} */
	let wrapper;
	const pages = /** @type {[string, string, boolean][]} */ ([
		["", "Home", true],
		["page1", "Page 1", true],
		["page2", "Page 2", false],
		["settings", "Settings", true]
	]);
</script>

<Meta
	title="Components/Navbar"
	argTypes={{
		visible: {
			options: [true, false],
			description: "Sets the visibility of the navbar",
			control: { type: "boolean" },
			defaultValue: true
		},
		main_page_name: {
			control: "text",
			description:
				"The name to display for the main page in the navbar. Set to false to use default 'Home'",
			name: "main_page_name",
			value: "Home"
		},
		value: {
			control: "object",
			description:
				"List of [route, name] tuples for additional navbar pages. These are added to the existing pages",
			name: "value",
Repository: gradio-app/gradio
File: gradio/cli/commands/__init__.py (Python)

Code before:
from .cli_env_info import print_environment_info
from .components import app as custom_component
from .deploy_space import deploy
from .hf_login import hf_login
from .openaichat import main as openaichat
from .reload import main as reload
from .sketch import launch as sketch
from .upload_mcp import main as upload_mcp

__all__ = [
    "deploy",
    "reload",
    "print_environment_info",
    "custom_component",
    "sketch",
    "upload_mcp",
    "hf_login",
    "openaichat",
]


Review comment (suggestion):
I think `load-chat` would be a better name @jyotiradityadav ! That way we can add `load` command later as well.

Code after:
from .cli_env_info import print_environment_info
from .components import app as custom_component
from .deploy_space import deploy
from .hf_login import hf_login
from .load import load_app
from .reload import main as reload
from .sketch import launch as sketch
from .upload_mcp import main as upload_mcp

__all__ = [
    "deploy",
    "reload",
    "print_environment_info",
    "custom_component",
    "sketch",
    "upload_mcp",
    "hf_login",
    "load_app",
]
Repository: gradio-app/gradio
File: demo/iframe_resizer/run.py (Python)

Code before:
import gradio as gr
import time


def greet():
    gr.Info("Warning in 1 second")
    time.sleep(1)
    gr.Warning("Error in 1 second")
    time.sleep(1)
    raise Exception("test")


with gr.Blocks() as demo:
    with gr.Tab("Accordions"):
        with gr.Row(height=1500):
            gr.Markdown("Scroll down to see UI.")
        greet_btn = gr.Button("Trigger toast")
        greet_btn.click(fn=greet)

        with gr.Accordion("Accordion"):
            gr.Markdown(
                """
    ## Accordion content
    ### Accordion content
    #### Accordion content
    ##### Accordion content
    ###### Accordion content
    """
            )
    with gr.Tab("Images"):
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")
        gr.Image(value="./cheetah.jpg")


Review comment (suggestion):
Nice @pngwn ! Let's add this demo to the [list](https://github.com/gradio-app/gradio/blob/03024f8cae5d1a35be8fe6a988ae3917eefba662/scripts/copy_demos.py#L15) of deployed spaces so we can test for regressions before we release.

In order for the demo to work you should change the value of all the images to be the full path to the cheetah file

Code after:
import gradio as gr
import time
import os


def greet():
    gr.Info("Warning in 1 second")
    time.sleep(1)
    gr.Warning("Error in 1 second")
    time.sleep(1)
    raise Exception("test")


im = os.path.join(os.path.dirname(__file__), "cheetah.jpg")

with gr.Blocks() as demo:
    with gr.Tab("Accordions"):
        with gr.Row(height=1500):
            gr.Markdown("Scroll down to see UI.")
        greet_btn = gr.Button("Trigger toast")
        greet_btn.click(fn=greet)

        with gr.Accordion("Accordion"):
            gr.Markdown(
                """
    ## Accordion content
    ### Accordion content
    #### Accordion content
    ##### Accordion content
    ###### Accordion content
    """
            )
    with gr.Tab("Images"):
        gr.Image(value=im)
        gr.Image(value=im)
        gr.Image(value=im)
        gr.Image(value=im)
        gr.Image(value=im)
        gr.Image(value=im)
        gr.Image(value=im)
        gr.Image(value=im)
        gr.Image(value=im)
Repository: gradio-app/gradio
File: demo/video_subtitle/run.py (Python)

Code before:
import gradio as gr
import os

a = os.path.join(os.path.dirname(__file__), "files/a.mp4")
b = os.path.join(os.path.dirname(__file__), "files/b.mp4")
s1 = os.path.join(os.path.dirname(__file__), "files/s1.srt")
s2 = os.path.join(os.path.dirname(__file__), "files/s2.vtt")
s3 = os.path.join(os.path.dirname(__file__), "files/s2.json")

def video_demo(video, subtitle=None):
    if subtitle is None:
        return video
    return gr.Video(label="Out", value=video, subtitles=subtitle.name)

demo = gr.Interface(
    fn=video_demo,
    inputs=[
        gr.Video(label="In", interactive=True),
        gr.File(label="Subtitle", file_types=[".srt", ".vtt", ".json"]),
    ],
    outputs=gr.Video(label="Out"),
    examples=[
        [a, s1],
        [b, s2],
        [a, s3],
    ],
)

if __name__ == "__main__":

Review comment (suggestion):
We should use `gr.load_video("a.mp4") etc to not have to copy over the files see #11989

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

def video_demo(video, subtitle=None):
    if subtitle is None:
        return video
    return gr.Video(label="Out", value=video, subtitles=subtitle.name)

demo = gr.Interface(
    fn=video_demo,
    inputs=[
        gr.Video(label="In", interactive=True),
        gr.File(label="Subtitle", file_types=[".srt", ".vtt", ".json"]),
    ],
    outputs=gr.Video(label="Out"),
    examples=[
        [get_video("a.mp4"), get_file("s1.srt")],
        [get_video("b.mp4"), get_file("s2.vtt")],
        [get_video("a.mp4"), get_file("s3.json")],
    ],
)

if __name__ == "__main__":
    demo.launch()
Repository: gradio-app/gradio
File: test/test_component_props.py (Python)

Code before:
"""Tests for the component props feature."""

import gradio as gr
from gradio import helpers


def test_special_args_detects_component_type_hints():
    """Test that special_args detects when a parameter is type-hinted with a component."""

    def func_with_component_hint(x: gr.Number):
        return x.value * 2

    def func_without_hint(x):
        return x * 2

    # Test that component type hint is detected
    _, _, _, component_prop_indices = helpers.special_args(func_with_component_hint)
    assert 0 in component_prop_indices

    # Test that no component type hint is detected
    _, _, _, component_prop_indices = helpers.special_args(func_without_hint)
    assert len(component_prop_indices) == 0


def test_special_args_creates_namespace_with_props():
    """Test that special_args creates a namespace object with component props."""

    def func(x: gr.Number):
        return x.value

    component_props = {
        0: {"value": 5, "minimum": 0, "maximum": 10, "label": "Test Number"}
    }

    inputs, _, _, _ = helpers.special_args(
        func, inputs=[5], component_props=component_props
    )

    # Check that the input is a SimpleNamespace with all props
    assert hasattr(inputs[0], "value")
    assert hasattr(inputs[0], "minimum")

Review comment (suggestion):
Remove inline comments here and in tests below
```suggestion
    # Test that component type hint is detected
```

Code after:
"""Tests for the component props feature."""

import gradio as gr
from gradio import helpers


def test_special_args_detects_component_type_hints():
    """Test that special_args detects when a parameter is type-hinted with a component."""

    def func_with_component_hint(x: gr.Number):
        return x.value * 2

    def func_without_hint(x):
        return x * 2

    # Test that component type hint is detected
    _, _, _, component_prop_indices = helpers.special_args(func_with_component_hint)
    assert 0 in component_prop_indices

    # Test that no component type hint is detected
    _, _, _, component_prop_indices = helpers.special_args(func_without_hint)
    assert len(component_prop_indices) == 0


def test_special_args_creates_namespace_with_props():
    """Test that special_args creates a namespace object with component props."""

    def func(x: gr.Number):
        return x.value

    component_props = {
        0: {"value": 5, "minimum": 0, "maximum": 10, "label": "Test Number"}
    }

    inputs, *_ = helpers.special_args(func, inputs=[5], component_props=component_props)

    # Check that the input is a SimpleNamespace with all props
    assert hasattr(inputs[0], "value")
    assert hasattr(inputs[0], "minimum")
    assert hasattr(inputs[0], "maximum")
    assert hasattr(inputs[0], "label")