ronantakizawa/codereview-bench huggingface dataset:
Repository: gradio-app/gradio
File: js/app/src/routes/[...catchall]/+page.ts (TypeScript)

Code before:

import { Client } from "@gradio/client";
import {
	create_components,
	type ComponentMeta,
	type Dependency
} from "@gradio/core";
import { get } from "svelte/store";
import type { Config } from "@gradio/client";

import Blocks from "@gradio/core/blocks";
import Login from "@gradio/core/login";

export async function load({
	url,
	data: { server, port, local_dev_mode, custom_path }
}): Promise<{
	Render: typeof Login | typeof Blocks;
	config: Config;
	api_url: string;
	layout: unknown;
	app: Client;
}> {
	const api_url =
		browser && !local_dev_mode
			? (location.origin + location.pathname).replace(/\/$/, "")
			: server;
	const deepLink = url.searchParams.get("deep_link");
	const app = await Client.connect(api_url, {
		with_null_state: true,
		events: ["data", "log", "status", "render"],
		query_params: deepLink ? { deep_link: deepLink } : undefined
	});

	if (!app.config) {
		throw new Error("No config found");
	}

	let page_config = app.get_url_config(url);

	const { create_layout, layout } = create_components(undefined);

	await create_layout({
		app,
		components: page_config.components,
		dependencies: page_config.dependencies,
		layout: page_config.layout,
		root: app.config.root + app.config.api_prefix,
		options: {
			fill_height: app.config.fill_height
		}

Review comment (suggestion):
we should use the URL constructor here and string concatenation as it is less error prone: `new URL(location.pathname, location.origin)`

Code after:

import { Client } from "@gradio/client";
import {
	create_components,
	type ComponentMeta,
	type Dependency
} from "@gradio/core";
import { get } from "svelte/store";
import type { Config } from "@gradio/client";

import Blocks from "@gradio/core/blocks";
import Login from "@gradio/core/login";

export async function load({
	url,
	data: { server, port, local_dev_mode, custom_path }
}): Promise<{
	Render: typeof Login | typeof Blocks;
	config: Config;
	api_url: string;
	layout: unknown;
	app: Client;
}> {
	const api_url =
		browser && !local_dev_mode ? new URL(".", location.href).href : server;
	const deepLink = url.searchParams.get("deep_link");
	const app = await Client.connect(api_url, {
		with_null_state: true,
		events: ["data", "log", "status", "render"],
		query_params: deepLink ? { deep_link: deepLink } : undefined
	});

	if (!app.config) {
		throw new Error("No config found");
	}

	let page_config = app.get_url_config(url);

	const { create_layout, layout } = create_components(undefined);

	await create_layout({
		app,
		components: page_config.components,
		dependencies: page_config.dependencies,
		layout: page_config.layout,
		root: app.config.root + app.config.api_prefix,
		options: {
			fill_height: app.config.fill_height
		}
	});
Repository: gradio-app/gradio
File: gradio/utils.py (Python)

Code before:
T = TypeVar("T")


def get_package_version() -> str:
    try:
        package_json_data = (
            pkgutil.get_data(__name__, "package.json").decode("utf-8").strip()  # type: ignore
        )
        package_data = json.loads(package_json_data)
        version = package_data.get("version", "")
        return version
    except Exception:
        return ""


def safe_get_lock() -> asyncio.Lock:
    """Get asyncio.Lock() without fear of getting an Exception.

    Needed because in reload mode we import the Blocks object outside
    the main thread.
    """
    try:
        asyncio.get_event_loop()
        return asyncio.Lock()
    except RuntimeError:
        return None  # type: ignore


def safe_get_stop_event() -> asyncio.Event:
    try:
        asyncio.get_event_loop()
        return asyncio.Event()
    except RuntimeError:
        return None  # type: ignore


class BaseReloader(ABC):
    @property
    @abstractmethod
    def running_app(self) -> App:
        pass

    def swap_blocks(self, demo: Blocks):
        assert self.running_app.blocks  # noqa: S101
        # Copy over the blocks to get new components and events but
        # not a new queue
        demo._queue = self.running_app.blocks._queue
        demo.has_launched = True
        demo.max_file_size = self.running_app.blocks.max_file_size
        demo.is_running = True
        self.running_app.state_holder.reset(demo)

Review comment (suggestion):
Before, we were returning `None`, which wasn't actually safe to do at all. It would just kick the can down the road and users would still see a `AttributeError: 'NoneType' object has no attribute 'wait'` later on

Code after:
T = TypeVar("T")


def get_package_version() -> str:
    try:
        package_json_data = (
            pkgutil.get_data(__name__, "package.json").decode("utf-8").strip()  # type: ignore
        )
        package_data = json.loads(package_json_data)
        version = package_data.get("version", "")
        return version
    except Exception:
        return ""


def safe_get_lock() -> asyncio.Lock:
    """Get asyncio.Lock() without fear of getting an Exception.

    Needed because in reload mode we import the Blocks object outside
    the main thread.
    """
    try:
        loop = asyncio.get_event_loop()
        if loop.is_closed():
            loop = asyncio.new_event_loop()
            asyncio.set_event_loop(loop)
        return asyncio.Lock()
    except RuntimeError:
        loop = asyncio.new_event_loop()
        asyncio.set_event_loop(loop)
        return asyncio.Lock()


def safe_get_stop_event() -> asyncio.Event:
    try:
        loop = asyncio.get_event_loop()
        if loop.is_closed():
            loop = asyncio.new_event_loop()
            asyncio.set_event_loop(loop)
        return asyncio.Event()
    except RuntimeError:
        loop = asyncio.new_event_loop()
        asyncio.set_event_loop(loop)
        return asyncio.Event()


class BaseReloader(ABC):
    @property
    @abstractmethod
    def running_app(self) -> App:
        pass
Repository: gradio-app/gradio
File: js/textbox/shared/utils.ts (TypeScript)

Code before:
import type { InputHTMLAttributes } from "./types";

export function convert_html_attributes(
	attrs: InputHTMLAttributes | null | undefined
): Record<string, string> {
	if (!attrs) return {};

	const result: Record<string, string> = {};

	const add_attr = (key: string, value: any): void => {
		if (value !== null && value !== undefined) {
			if (typeof value === "boolean") {
				result[key] = value ? "true" : "false";
			} else {
				result[key] = String(value);
			}
		}
	};

	const add_autocorrect = (value: boolean | null | undefined): void => {
		if (value !== null && value !== undefined) {
			result["autocorrect"] = value ? "on" : "off";
		}
	};

	const add_autocapitalize = (value: string | null | undefined): void => {
		if (value !== null && value !== undefined) {
			const mapping: Record<string, string> = {
				sentences: "sentences",
				words: "words",
				characters: "characters",
				off: "off",
				none: "off",
				false: "off",
				true: "sentences"
			};
			result["autocapitalize"] = mapping[value.toLowerCase()] || value;
		}
	};

	for (const [key, value] of Object.entries(attrs)) {
		if (key === "autocapitalize") {
			add_autocapitalize(value);
		} else if (key === "autocorrect") {
			add_autocorrect(value);
		} else {
			const htmlKey = key.replace(/_/g, "-");
			add_attr(htmlKey, value);
		}
	}


Review comment (refactor):
This seems unnecessary and `true` isn't a valid value, so I don't think we should allow it. We shouldn't abstract over these attributes. 

Code after:
import type { InputHTMLAttributes } from "./types";

export function convert_html_attributes(
	attrs: InputHTMLAttributes | null | undefined
): Record<string, string> {
	if (!attrs) return {};

	const result: Record<string, string> = {};

	const add_attr = (key: string, value: any): void => {
		if (value !== null && value !== undefined) {
			if (typeof value === "boolean") {
				result[key] = value ? "true" : "false";
			} else {
				result[key] = String(value);
			}
		}
	};

	for (const [key, value] of Object.entries(attrs)) {
		const html_key = key.replace(/_/g, "-");
		add_attr(html_key, value);
	}

	return result;
}
Repository: gradio-app/gradio
File: js/textbox/shared/utils.ts (TypeScript)

Code before:
import type { InputHTMLAttributes } from "./types";

export function convert_html_attributes(
	attrs: InputHTMLAttributes | null | undefined
): Record<string, string> {
	if (!attrs) return {};

	const result: Record<string, string> = {};

	const add_attr = (key: string, value: any): void => {
		if (value !== null && value !== undefined) {
			if (typeof value === "boolean") {
				result[key] = value ? "true" : "false";
			} else {
				result[key] = String(value);
			}
		}
	};

	const add_autocorrect = (value: boolean | null | undefined): void => {
		if (value !== null && value !== undefined) {
			result["autocorrect"] = value ? "on" : "off";
		}
	};

	const add_autocapitalize = (value: string | null | undefined): void => {
		if (value !== null && value !== undefined) {
			const mapping: Record<string, string> = {
				sentences: "sentences",
				words: "words",
				characters: "characters",
				off: "off",
				none: "off",
				false: "off",
				true: "sentences"
			};
			result["autocapitalize"] = mapping[value.toLowerCase()] || value;
		}
	};

	for (const [key, value] of Object.entries(attrs)) {
		if (key === "autocapitalize") {
			add_autocapitalize(value);
		} else if (key === "autocorrect") {
			add_autocorrect(value);

Review comment (suggestion):
Same for this. The value should already be on or off, we shouldn't do additional validation. Types in the dataclass are enough imo. 

Code after:
import type { InputHTMLAttributes } from "./types";

export function convert_html_attributes(
	attrs: InputHTMLAttributes | null | undefined
): Record<string, string> {
	if (!attrs) return {};

	const result: Record<string, string> = {};

	const add_attr = (key: string, value: any): void => {
		if (value !== null && value !== undefined) {
			if (typeof value === "boolean") {
				result[key] = value ? "true" : "false";
			} else {
				result[key] = String(value);
			}
		}
	};

	for (const [key, value] of Object.entries(attrs)) {
		const html_key = key.replace(/_/g, "-");
		add_attr(html_key, value);
	}

	return result;
}
Repository: gradio-app/gradio
File: js/textbox/shared/types.ts (TypeScript)

Code before:
export interface InputHTMLAttributes {
	autocapitalize?: string | null;
	autocorrect?: boolean | null;
	spellcheck?: boolean | null;
	autocomplete?: string | null;
	tabindex?: number | null;
	enterkeyhint?: string | null;
	lang?: string | null;
	aria_label?: string | null;
	aria_describedby?: string | null;
	aria_placeholder?: string | null;
}


Review comment (suggestion):
```suggestion
	autocomplete?: "off" | "on" | null;
```

Code after:
export interface InputHTMLAttributes {
	autocapitalize?:
		| "off"
		| "none"
		| "on"
		| "sentences"
		| "words"
		| "characters"
		| null;
	autocorrect?: "on" | "off" | null;
	spellcheck?: boolean | null;
	autocomplete?: string | null;
	tabindex?: number | null;
	enterkeyhint?:
		| "enter"
		| "done"
		| "go"
		| "next"
		| "previous"
		| "search"
		| "send"
		| null;
	lang?: string | null;
	aria_label?: string | null;
	aria_describedby?: string | null;
	aria_placeholder?: string | null;
}
Repository: gradio-app/gradio
File: js/textbox/shared/types.ts (TypeScript)

Code before:
export interface InputHTMLAttributes {
	autocapitalize?: string | null;
	autocorrect?: boolean | null;
	spellcheck?: boolean | null;
	autocomplete?: string | null;
	tabindex?: number | null;
	enterkeyhint?: string | null;
	lang?: string | null;
	aria_label?: string | null;
	aria_describedby?: string | null;
	aria_placeholder?: string | null;
}


Review comment (suggestion):
```suggestion
	autocapitalize?: "off" | "none" | "on" | "sentences" | "words" | "characters" | null;
```

Code after:
export interface InputHTMLAttributes {
	autocapitalize?:
		| "off"
		| "none"
		| "on"
		| "sentences"
		| "words"
		| "characters"
		| null;
	autocorrect?: "on" | "off" | null;
	spellcheck?: boolean | null;
	autocomplete?: string | null;
	tabindex?: number | null;
	enterkeyhint?:
		| "enter"
		| "done"
		| "go"
		| "next"
		| "previous"
		| "search"
		| "send"
		| null;
	lang?: string | null;
	aria_label?: string | null;
	aria_describedby?: string | null;
	aria_placeholder?: string | null;
}
Repository: gradio-app/gradio
File: js/textbox/shared/types.ts (TypeScript)

Code before:
export interface InputHTMLAttributes {
	autocapitalize?: string | null;
	autocorrect?: boolean | null;
	spellcheck?: boolean | null;
	autocomplete?: string | null;
	tabindex?: number | null;
	enterkeyhint?: string | null;
	lang?: string | null;
	aria_label?: string | null;
	aria_describedby?: string | null;
	aria_placeholder?: string | null;
}


Review comment (suggestion):
```suggestion
	autocorrect?: "on" | "off" | null;
```

Code after:
export interface InputHTMLAttributes {
	autocapitalize?:
		| "off"
		| "none"
		| "on"
		| "sentences"
		| "words"
		| "characters"
		| null;
	autocorrect?: "on" | "off" | null;
	spellcheck?: boolean | null;
	autocomplete?: string | null;
	tabindex?: number | null;
	enterkeyhint?:
		| "enter"
		| "done"
		| "go"
		| "next"
		| "previous"
		| "search"
		| "send"
		| null;
	lang?: string | null;
	aria_label?: string | null;
	aria_describedby?: string | null;
	aria_placeholder?: string | null;
}
Repository: gradio-app/gradio
File: js/textbox/shared/types.ts (TypeScript)

Code before:
export interface InputHTMLAttributes {
	autocapitalize?: string | null;
	autocorrect?: boolean | null;
	spellcheck?: boolean | null;
	autocomplete?: string | null;
	tabindex?: number | null;
	enterkeyhint?: string | null;
	lang?: string | null;
	aria_label?: string | null;
	aria_describedby?: string | null;
	aria_placeholder?: string | null;
}


Review comment (suggestion):
```suggestion
	enterkeyhint?: "enter" | "done" | "go" | "next" | "previous" | "search" | "send" | null;
```

Code after:
export interface InputHTMLAttributes {
	autocapitalize?:
		| "off"
		| "none"
		| "on"
		| "sentences"
		| "words"
		| "characters"
		| null;
	autocorrect?: "on" | "off" | null;
	spellcheck?: boolean | null;
	autocomplete?: string | null;
	tabindex?: number | null;
	enterkeyhint?:
		| "enter"
		| "done"
		| "go"
		| "next"
		| "previous"
		| "search"
		| "send"
		| null;
	lang?: string | null;
	aria_label?: string | null;
	aria_describedby?: string | null;
	aria_placeholder?: string | null;
}
Repository: gradio-app/gradio
File: client/js/src/helpers/init_helpers.ts (TypeScript)

Code before:
	} else if (endpoint) {
		let config_url = join_urls(
			endpoint,
			this.deep_link ? CONFIG_URL + "?deep_link=" + this.deep_link : CONFIG_URL
		);

		const response = await this.fetch(config_url, {
			headers,
			credentials: "include"
		});

		return handleConfigResponse(response, endpoint, !!this.options.auth);
	}

	throw new Error(CONFIG_ERROR_MSG);
}

async function handleConfigResponse(
	response: Response,
	endpoint: string,
	authorized: boolean
): Promise<Config> {
	if (response?.status === 401 && !authorized) {
		const error_data = await response.json();
		const auth_message = error_data?.detail?.auth_message;
		throw new Error(auth_message || "");
	} else if (response?.status === 401 && authorized) {
		throw new Error(INVALID_CREDENTIALS_MSG);
	}

	if (response?.status === 200) {
		let config = await response.json();
		config.root = endpoint;
		config.dependencies?.forEach((dep: any, i: number) => {
			if (dep.id === undefined) {
				dep.id = i;
			}
		});
		return config;
	} else if (response?.status === 401) {
		throw new Error(UNAUTHORIZED_MSG);
	}

	throw new Error(CONFIG_ERROR_MSG);
}

export async function resolve_cookies(this: Client): Promise<void> {
	const { http_protocol, host } = await process_endpoint(
		this.app_reference,
		this.options.hf_token
	);

Review comment (bug):
I think we could fall back to `MISSING_CREDENTIALS_MSG` here instead of empty string, this will also get rid of the failing test i think.

Code after:
	} else if (endpoint) {
		let config_url = join_urls(
			endpoint,
			this.deep_link ? CONFIG_URL + "?deep_link=" + this.deep_link : CONFIG_URL
		);

		const response = await this.fetch(config_url, {
			headers,
			credentials: "include"
		});

		return handleConfigResponse(response, endpoint, !!this.options.auth);
	}

	throw new Error(CONFIG_ERROR_MSG);
}

async function handleConfigResponse(
	response: Response,
	endpoint: string,
	authorized: boolean
): Promise<Config> {
	if (response?.status === 401 && !authorized) {
		const error_data = await response.json();
		const auth_message = error_data?.detail?.auth_message;
		throw new Error(auth_message || MISSING_CREDENTIALS_MSG);
	} else if (response?.status === 401 && authorized) {
		throw new Error(INVALID_CREDENTIALS_MSG);
	}

	if (response?.status === 200) {
		let config = await response.json();
		config.root = endpoint;
		config.dependencies?.forEach((dep: any, i: number) => {
			if (dep.id === undefined) {
				dep.id = i;
			}
		});
		return config;
	} else if (response?.status === 401) {
		throw new Error(UNAUTHORIZED_MSG);
	}

	throw new Error(CONFIG_ERROR_MSG);
}

export async function resolve_cookies(this: Client): Promise<void> {
	const { http_protocol, host } = await process_endpoint(
		this.app_reference,
		this.options.hf_token
	);
Repository: gradio-app/gradio
File: gradio/cli/cli.py (Python)

Code before:
    "deploy",
    help="Deploy a Gradio app to Spaces. Must be called within the directory you would like to deploy.",
)(deploy)
app.command("deploy-discord", help="Deploy a Gradio app to Discord.")(
    deploy_discord.main
)
app.command("sketch", help="Open the Sketch app to design a Gradio app.")(sketch)


def cli():
    args = sys.argv[1:]
    if len(args) == 0:
        raise ValueError("No file specified.")
    if args[0] in {"deploy", "environment", "deploy-discord", "sketch"}:
        app()
    elif args[0] in {"cc", "component"}:
        sys.argv = sys.argv[1:]
        custom_component()
    elif args[0] in {"build", "dev", "create", "show", "publish", "install"}:
        try:
            error = f"gradio {args[0]} is not a valid command. Did you mean `gradio cc {args[0]}` or `gradio component {args[0]}`?."
            raise ValueError(error)
        except ValueError:
            console = Console()
            console.print_exception()
    elif args[0] in {"hf-upload-mcp"}:
        hf_upload_mcp(args[1], args[2])
    else:
        typer.run(reload)


Review comment (suggestion):
I think we shoudl rename `hf-upload-mcp` to just `upload-mcp` because it doesn't make sense if the gradio app is running locally or hosted on a server other than hf spaces.

Code after:
    "deploy",
    help="Deploy a Gradio app to Spaces. Must be called within the directory you would like to deploy.",
)(deploy)
app.command("deploy-discord", help="Deploy a Gradio app to Discord.")(
    deploy_discord.main
)
app.command("sketch", help="Open the Sketch app to design a Gradio app.")(sketch)


def cli():
    args = sys.argv[1:]
    if len(args) == 0:
        raise ValueError("No file specified.")
    if args[0] in {"deploy", "environment", "deploy-discord", "sketch"}:
        app()
    elif args[0] in {"cc", "component"}:
        sys.argv = sys.argv[1:]
        custom_component()
    elif args[0] in {"build", "dev", "create", "show", "publish", "install"}:
        try:
            error = f"gradio {args[0]} is not a valid command. Did you mean `gradio cc {args[0]}` or `gradio component {args[0]}`?."
            raise ValueError(error)
        except ValueError:
            console = Console()
            console.print_exception()
    elif args[0] in {"upload-mcp"}:
        upload_mcp(args[1], args[2])
    else:
        typer.run(reload)
Repository: gradio-app/gradio
File: gradio/cli/commands/upload_mcp.py (Python)

Code before:
def main(url_or_space_id: str, source_directory: str):
    import requests
    from gradio_client.utils import is_http_url_like
    from huggingface_hub import space_info
    from mcp.server.fastmcp import FastMCP

    from gradio.utils import abspath, is_in_or_equal

    source_path = abspath(source_directory)

    mcp = FastMCP("upload-mcp")

    if is_http_url_like(url_or_space_id):
        url = url_or_space_id.rstrip("/")
    else:
        url = f"https://{space_info(url_or_space_id).subdomain}.hf.space"

    @mcp.tool()
    def upload_to_space(file: str) -> str:
        """Upload a local file to a Hugging Face Space.
        Arguments:
            file: A complete, absolutepath to a local file to upload.
        Returns:
            A URL to the uploaded file.
        """

        target_path = abspath(file)

        if not is_in_or_equal(target_path, source_path):
            raise ValueError(f"File {file} is not in {source_path}")

        with open(target_path, "rb") as f:
            response = requests.post(f"{url}/gradio_api/upload", files={"files": f})
        response.raise_for_status()
        result = response.json()[0]
        return f"{url}/gradio_api/file={result}"

    mcp.run(transport="stdio")


if __name__ == "__main__":
    import typer

    typer.run(main)


Review comment (suggestion):
Let's rename this tool to just `upload_files` to match the snippet we show on the mcp docs page. As it stands, it might be confusing to see it called `upload_to_space`:

<img width="401" height="429" alt="image" src="https://github.com/user-attachments/assets/f6620b66-7539-485c-aa31-e62b6953baa5" />


Code after:
def main(url_or_space_id: str, source_directory: str):
    import requests
    from gradio_client.utils import is_http_url_like
    from huggingface_hub import space_info
    from mcp.server.fastmcp import FastMCP

    from gradio.utils import abspath, is_in_or_equal

    source_path = abspath(source_directory)

    mcp = FastMCP("upload-mcp")

    if is_http_url_like(url_or_space_id):
        url = url_or_space_id.rstrip("/")
    else:
        url = f"https://{space_info(url_or_space_id).subdomain}.hf.space"

    @mcp.tool()
    def upload_file_to_gradio_or_hf(file: str) -> str:
        """Upload a local file to a lication.
        Arguments:
            file: A complete, absolutepath to a local file to upload.
        Returns:
            A URL to the uploaded file.
        """

        target_path = abspath(file)

        if not is_in_or_equal(target_path, source_path):
            raise ValueError(f"File {file} is not in {source_path}")

        with open(target_path, "rb") as f:
            response = requests.post(f"{url}/gradio_api/upload", files={"files": f})
        response.raise_for_status()
        result = response.json()[0]
        return f"{url}/gradio_api/file={result}"

    mcp.run(transport="stdio")


if __name__ == "__main__":
    import typer

    typer.run(main)