ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: api/src/services/server.ts (TypeScript)

Code before:
				'default_theme_light',
				'default_theme_dark',
				'theme_light_overrides',
				'theme_dark_overrides',
				'default_language',
				'public_foreground',
				'public_background.id',
				'public_background.type',
				'public_favicon',
				'public_note',
				'custom_css',
				'public_registration',
				'public_registration_verify_email',
			],
		});

		info['project'] = projectInfo;

		info['setupCompleted'] = setupComplete;

		if (this.accountability?.user) {
			info['mcp_enabled'] = toBoolean(env['MCP_ENABLED'] ?? true);
			info['ai_enabled'] = toBoolean(env['AI_ENABLED'] ?? true);

			info['files'] = {
				mimeTypeAllowList: env['FILES_MIME_TYPE_ALLOW_LIST'] ?? '*/*',
			};

			if (env['RATE_LIMITER_ENABLED']) {
				info['rateLimit'] = {
					points: env['RATE_LIMITER_POINTS'],
					duration: env['RATE_LIMITER_DURATION'],
				};
			} else {
				info['rateLimit'] = false;
			}

			if (env['RATE_LIMITER_GLOBAL_ENABLED']) {
				info['rateLimitGlobal'] = {
					points: env['RATE_LIMITER_GLOBAL_POINTS'],
					duration: env['RATE_LIMITER_GLOBAL_DURATION'],
				};
			} else {
				info['rateLimitGlobal'] = false;
			}

			info['extensions'] = {
				limit: env['EXTENSIONS_LIMIT'] ?? null,
			};

			info['queryLimit'] = {

Review comment (suggestion):
The nullish coalescing is never triggered since the `env` is populated with default values in [`create-env.ts`](https://github.com/directus/directus/blob/e95d6df6f7ee081e7b3371db2c87e916fd0c5dba/packages/env/src/lib/create-env.ts#L22) and already defaults to [`*/*`](https://github.com/directus/directus/blob/e95d6df6f7ee081e7b3371db2c87e916fd0c5dba/packages/env/src/constants/defaults.ts#L196)

Code after:
				'default_theme_light',
				'default_theme_dark',
				'theme_light_overrides',
				'theme_dark_overrides',
				'default_language',
				'public_foreground',
				'public_background.id',
				'public_background.type',
				'public_favicon',
				'public_note',
				'custom_css',
				'public_registration',
				'public_registration_verify_email',
			],
		});

		info['project'] = projectInfo;

		info['setupCompleted'] = setupComplete;

		if (this.accountability?.user) {
			info['mcp_enabled'] = toBoolean(env['MCP_ENABLED'] ?? true);
			info['ai_enabled'] = toBoolean(env['AI_ENABLED'] ?? true);

			info['files'] = {
				mimeTypeAllowList: env['FILES_MIME_TYPE_ALLOW_LIST'],
			};

			if (env['RATE_LIMITER_ENABLED']) {
				info['rateLimit'] = {
					points: env['RATE_LIMITER_POINTS'],
					duration: env['RATE_LIMITER_DURATION'],
				};
			} else {
				info['rateLimit'] = false;
			}

			if (env['RATE_LIMITER_GLOBAL_ENABLED']) {
				info['rateLimitGlobal'] = {
					points: env['RATE_LIMITER_GLOBAL_POINTS'],
					duration: env['RATE_LIMITER_GLOBAL_DURATION'],
				};
			} else {
				info['rateLimitGlobal'] = false;
			}

			info['extensions'] = {
				limit: env['EXTENSIONS_LIMIT'] ?? null,
			};

			info['queryLimit'] = {
Repository: directus/directus
File: api/src/controllers/files.ts (TypeScript)

Code before:
				};
			} else {
				const key = Array.isArray(keys) ? keys[0]! : keys;
				const record = await service.readOne(key, req.sanitizedQuery);

				res.locals['payload'] = {
					data: record,
				};
			}
		} catch (error: any) {
			if (isDirectusError(error, ErrorCode.Forbidden)) {
				return next();
			}

			throw error;
		}

		return next();
	}),
	respond,
);

const importSchema = Joi.object({
	url: Joi.string().required(),
	data: Joi.object(),
	allowedMimeTypes: Joi.array().items(Joi.string()),
});

router.post(
	'/import',
	asyncHandler(async (req, res, next) => {
		const { error } = importSchema.validate(req.body);

		if (error) {
			throw new InvalidPayloadError({ reason: error.message });
		}

		const service = new FilesService({
			accountability: req.accountability,
			schema: req.schema,
		});

		const primaryKey = await service.importOne(req.body.url, req.body.data, req.body.allowedMimeTypes);

		try {
			const record = await service.readOne(primaryKey, req.sanitizedQuery);
			res.locals['payload'] = { data: record || null };
		} catch (error: any) {
			if (isDirectusError(error, ErrorCode.Forbidden)) {
				return next();
			}

Review comment (security):
From a security standpoint it feels a bit strange to send an "allowList" with the request itself as they should not be user-controlled. Since this is not a security measure but more of "filter" I think we should name it as such to prevent confusion about what this property does.
Something like `MimeTypeFilter` or `filterMimeType` perhaps 🤔 

Code after:
				};
			} else {
				const key = Array.isArray(keys) ? keys[0]! : keys;
				const record = await service.readOne(key, req.sanitizedQuery);

				res.locals['payload'] = {
					data: record,
				};
			}
		} catch (error: any) {
			if (isDirectusError(error, ErrorCode.Forbidden)) {
				return next();
			}

			throw error;
		}

		return next();
	}),
	respond,
);

const importSchema = Joi.object({
	url: Joi.string().required(),
	data: Joi.object(),
	options: Joi.object({ filterMimeType: Joi.array().items(Joi.string()) }),
});

router.post(
	'/import',
	asyncHandler(async (req, res, next) => {
		const { error } = importSchema.validate(req.body);

		if (error) {
			throw new InvalidPayloadError({ reason: error.message });
		}

		const service = new FilesService({
			accountability: req.accountability,
			schema: req.schema,
		});

		const primaryKey = await service.importOne(req.body.url, req.body.data, req.body.options);

		try {
			const record = await service.readOne(primaryKey, req.sanitizedQuery);
			res.locals['payload'] = { data: record || null };
		} catch (error: any) {
			if (isDirectusError(error, ErrorCode.Forbidden)) {
				return next();
			}
Repository: directus/directus
File: api/src/services/files.ts (TypeScript)

Code before:
			emitter.emitAction(
				'files.upload',
				{
					payload,
					key: primaryKey,
					collection: this.collection,
				},
				{
					database: this.knex,
					schema: this.schema,
					accountability: this.accountability,
				},
			);
		}

		return primaryKey;
	}

	/**
	 * Extract metadata from a buffer's content
	 */

	/**
	 * Import a single file from an external URL
	 */
	async importOne(importURL: string, body: Partial<File>, allowedMimeTypes?: string[]): Promise<PrimaryKey> {
		if (this.accountability) {
			await validateAccess(
				{
					accountability: this.accountability,
					action: 'create',
					collection: 'directus_files',
				},
				{
					knex: this.knex,
					schema: this.schema,
				},
			);
		}

		let fileResponse;

		try {
			const axios = await getAxios();

			fileResponse = await axios.get<Readable>(encodeURL(importURL), {
				responseType: 'stream',
				decompress: false,
			});
		} catch (error: any) {
			logger.warn(`Couldn't fetch file from URL "${importURL}"${error.message ? `: ${error.message}` : ''}`);

Review comment (suggestion):
```suggestion
	async importOne(importURL: string, body: Partial<File>, options?: { allowedMimeTypes?: string[] }): Promise<PrimaryKey> {
```
Instead of adding a dedicated mimetype argument to this function perhaps we can use an `options` object instead so we can easily add extra flags in the future if needed (perhaps file size restrictions or similar).

Code after:
			emitter.emitAction(
				'files.upload',
				{
					payload,
					key: primaryKey,
					collection: this.collection,
				},
				{
					database: this.knex,
					schema: this.schema,
					accountability: this.accountability,
				},
			);
		}

		return primaryKey;
	}

	/**
	 * Extract metadata from a buffer's content
	 */

	/**
	 * Import a single file from an external URL
	 */
	async importOne(
		importURL: string,
		body: Partial<File>,
		options: { filterMimeType?: string[] } = {},
	): Promise<PrimaryKey> {
		if (this.accountability) {
			await validateAccess(
				{
					accountability: this.accountability,
					action: 'create',
					collection: 'directus_files',
				},
				{
					knex: this.knex,
					schema: this.schema,
				},
			);
		}

		let fileResponse;

		try {
			const axios = await getAxios();

			fileResponse = await axios.get<Readable>(encodeURL(importURL), {
				responseType: 'stream',
Repository: directus/directus
File: app/src/views/private/private-view/stores/sidebar.ts (TypeScript)

Code before:
import { createEventHook, useLocalStorage } from '@vueuse/core';
import { defineStore } from 'pinia';
import { computed, ref, watch } from 'vue';

export const useSidebarStore = defineStore('sidebar-store', () => {
	const collapsed = useLocalStorage('sidebar-collapsed', false);

	const DEFAULT_SIZE = 370;
	const storedSize = useLocalStorage('sidebar-size', DEFAULT_SIZE);

	const size = computed({
		get() {
			const val = storedSize.value;
			return Number.isFinite(val) ? val : DEFAULT_SIZE;
		},
		set(val: number) {
			if (Number.isFinite(val)) storedSize.value = val;
		},
	});

	const activeAccordionItem = ref<string>();

	watch(activeAccordionItem, (newActiveItem) => {
		if (newActiveItem) expand();
	});

	watch(collapsed, (newCollapsed) => {
		if (newCollapsed) {
			activeAccordionItem.value = undefined;
			collapseHook.trigger();
		} else {
			expandHook.trigger();
		}
	});

	const collapseHook = createEventHook();
	const expandHook = createEventHook();

	const collapse = () => {

Review comment (suggestion):
non-blocking:

Maybe we should reset the stored value to `DEFAULT_SIZE` when an invalidad vlaue is detected? So corrupted localstorage is self-healed

 ```ts
     get() {
       const val = storedSize.value;
       if (!Number.isFinite(val)) {
         storedSize.value = DEFAULT_SIZE;
         return DEFAULT_SIZE;
       }
       return val;
     }
     ```

Code after:
import { createEventHook, useLocalStorage } from '@vueuse/core';
import { defineStore } from 'pinia';
import { computed, ref, watch } from 'vue';

export const useSidebarStore = defineStore('sidebar-store', () => {
	const collapsed = useLocalStorage('sidebar-collapsed', false);

	const DEFAULT_SIZE = 370;
	const storedSize = useLocalStorage('sidebar-size', DEFAULT_SIZE);

	const size = computed({
		get() {
			const val = storedSize.value;

			if (!Number.isFinite(val)) {
				storedSize.value = DEFAULT_SIZE;
				return DEFAULT_SIZE;
			}

			return val;
		},
		set(val: number) {
			if (Number.isFinite(val)) storedSize.value = val;
		},
	});

	const activeAccordionItem = ref<string>();

	watch(activeAccordionItem, (newActiveItem) => {
		if (newActiveItem) expand();
	});

	watch(collapsed, (newCollapsed) => {
		if (newCollapsed) {
			activeAccordionItem.value = undefined;
			collapseHook.trigger();
		} else {
			expandHook.trigger();
		}
Repository: directus/directus
File: app/src/modules/deployment/index.ts (TypeScript)

Code before:
import { defineModule } from '@directus/extensions';
import { useDeploymentNavigation } from './composables/use-deployment-navigation';
import DeploymentOverview from './routes/overview.vue';
import DeploymentProviderDashboard from './routes/provider/dashboard.vue';
import DeploymentProviderRun from './routes/provider/run.vue';
import DeploymentProviderRuns from './routes/provider/runs.vue';
import DeploymentProviderSettings from './routes/provider/settings.vue';

function ensureNavLoaded() {
	useDeploymentNavigation().fetch();
}

export default defineModule({
	id: 'deployments',
	name: '$t:deployment.deployment',
	icon: 'rocket_launch',
	routes: [
		{
			name: 'deployments-overview',
			path: '',
			component: DeploymentOverview,
			beforeEnter: ensureNavLoaded,
		},
		{
			name: 'deployments-provider-dashboard',
			path: ':provider',
			component: DeploymentProviderDashboard,
			props: true,
			beforeEnter: ensureNavLoaded,
		},
		{
			name: 'deployments-provider-settings',
			path: ':provider/settings',
			component: DeploymentProviderSettings,
			props: true,

Review comment (suggestion):
This probably should be returned, since `beforeEnter` awaits a Promise. Else, the router sees `undefined`  and render the component before the data loads.

```suggestion
	return useDeploymentNavigation().fetch();
```

Code after:
import { defineModule } from '@directus/extensions';
import { useDeploymentNavigation } from './composables/use-deployment-navigation';
import DeploymentOverview from './routes/overview.vue';
import DeploymentProviderDashboard from './routes/provider/dashboard.vue';
import DeploymentProviderRun from './routes/provider/run.vue';
import DeploymentProviderRuns from './routes/provider/runs.vue';
import DeploymentProviderSettings from './routes/provider/settings.vue';

function ensureNavLoaded() {
	return useDeploymentNavigation().fetch();
}

export default defineModule({
	id: 'deployments',
	name: '$t:deployment.deployment',
	icon: 'rocket_launch',
	routes: [
		{
			name: 'deployments-overview',
			path: '',
			component: DeploymentOverview,
			beforeEnter: ensureNavLoaded,
		},
		{
			name: 'deployments-provider-dashboard',
			path: ':provider',
			component: DeploymentProviderDashboard,
			props: true,
			beforeEnter: ensureNavLoaded,
		},
		{
			name: 'deployments-provider-settings',
			path: ':provider/settings',
			component: DeploymentProviderSettings,
			props: true,
Repository: directus/directus
File: api/src/deployment/drivers/netlify.ts (TypeScript)

Code before:
	}

	private mapSiteBase(site: NetlifySite): Project {
		const result: Project = {
			id: site.id!,
			name: site.name!,
			deployable: Boolean(site.build_settings?.provider && site.build_settings?.repo_url),
		};

		// Use custom domain if available, otherwise ssl_url or url
		if (site.custom_domain) {
			result.url = `https://${site.custom_domain}`;
		} else if (site.ssl_url) {
			result.url = site.ssl_url;
		} else if (site.url) {
			result.url = site.url;
		}

		return result;
	}

	async listProjects(): Promise<Project[]> {
		const allSites: any[] = [];
		const perPage = 100;
		let hasMore = true;

		for (let page = 1; hasMore; page++) {
			const params: Record<string, string> = { per_page: String(perPage), page: String(page) };

			const response = await this.handleApiError((api) => {
				return this.options.account_slug
					? api.listSitesForAccount({
							account_slug: this.options.account_slug,
							...params,
						})
					: api.listSites(params);
			});

			allSites.push(...response);
			hasMore = response.length >= perPage;
		}

		return allSites.map((site) => this.mapSiteBase(site));
	}

	async getProject(projectId: string): Promise<Project> {
		const site = await this.handleApiError((api) => api.getSite({ siteId: projectId }));
		const result = this.mapSiteBase(site);

		// Add published deploy info if available
		if (site.published_deploy) {

Review comment (bug):
The Vercel pagination got a test but Netlify didn't — mind adding one? The termination condition (`response.length >= perPage`) is worth covering, especially the edge case where the total is an exact multiple of the page size.

Also minor: `any[]` here where Vercel uses `Project[]` — could type this to match.

Code after:
		}
	}

	async testConnection(): Promise<void> {
		await this.handleApiError((api) => api.listSites({ per_page: 1 }));
	}

	private mapSiteBase(site: NetlifySite): Project {
		const result: Project = {
			id: site.id!,
			name: site.name!,
			deployable: Boolean(site.build_settings?.provider && site.build_settings?.repo_url),
		};

		// Use custom domain if available, otherwise ssl_url or url
		if (site.custom_domain) {
			result.url = `https://${site.custom_domain}`;
		} else if (site.ssl_url) {
			result.url = site.ssl_url;
		} else if (site.url) {
			result.url = site.url;
		}

		return result;
	}

	async listProjects(): Promise<Project[]> {
		const allSites: NetlifySite[] = [];
		const perPage = 100;
		let hasMore = true;

		for (let page = 1; hasMore; page++) {
			const params: Record<string, string> = { per_page: String(perPage), page: String(page) };

			const response = await this.handleApiError((api) => {
				return this.options.account_slug
					? api.listSitesForAccount({
							account_slug: this.options.account_slug,
							...params,
						})
					: api.listSites(params);
			});

			allSites.push(...response);
			hasMore = response.length >= perPage;
		}

		return allSites.map((site) => this.mapSiteBase(site));
	}

	async getProject(projectId: string): Promise<Project> {
Repository: directus/directus
File: app/src/interfaces/tags/tags.vue (Vue)

Code before:
		alphabetize?: boolean;
		iconLeft?: string;
		iconRight?: string;
		presets?: string[];
		allowCustom?: boolean;
		direction?: string;
		rawEditorEnabled?: boolean;
	}>(),
	{
		iconRight: 'local_offer',
		allowCustom: true,
	},
);

const emit = defineEmits(['input']);

const presetVals = computed<string[]>(() => {
	if (props.presets !== undefined) return processArray(props.presets);
	return [];
});

function isStringValue(val: unknown): val is string {
	return props.rawEditorEnabled === true && typeof val === 'string';
}

const isVariableMode = ref(isStringValue(props.value));

const selectedValsLocal = ref<string[]>(Array.isArray(props.value) ? processArray(props.value) : []);

watch(
	() => props.value,
	(newVal) => {
		isVariableMode.value = isStringValue(newVal);

		if (Array.isArray(newVal)) {
			selectedValsLocal.value = processArray(newVal);
		}

		if (newVal === null) selectedValsLocal.value = [];
	},
);

const selectedVals = computed<string[]>(() => {
	let vals = processArray(selectedValsLocal.value);

	if (!props.allowCustom) {
		vals = vals.filter((val) => presetVals.value.includes(val));
	}

	return vals;
});

Review comment (suggestion):
Let’s use a computed to follow common patterns and ensure reactivity does not break.

```suggestion
const isVariableMode = computed(() => props.rawEditorEnabled === true && typeof props.value === 'string';);
```

Code after:
		alphabetize?: boolean;
		iconLeft?: string;
		iconRight?: string;
		presets?: string[];
		allowCustom?: boolean;
		direction?: string;
		rawEditorEnabled?: boolean;
	}>(),
	{
		iconRight: 'local_offer',
		allowCustom: true,
	},
);

const emit = defineEmits(['input']);

const presetVals = computed<string[]>(() => {
	if (props.presets !== undefined) return processArray(props.presets);
	return [];
});

// Variable mode: when `rawEditorEnabled` is true and the value is a string rather than an array.
// This happens in contexts like Flows and Insights where the field value can be a dynamic
// variable expression (e.g. "{{$trigger.body.tags}}") instead of an array of tags.
const isVariableMode = computed(() => props.rawEditorEnabled === true && typeof props.value === 'string');

const selectedValsLocal = ref<string[]>(Array.isArray(props.value) ? processArray(props.value) : []);

watch(
	() => props.value,
	(newVal) => {
		if (Array.isArray(newVal)) {
			selectedValsLocal.value = processArray(newVal);
		}

		if (newVal === null) selectedValsLocal.value = [];
	},
);

const selectedVals = computed<string[]>(() => {
	let vals = processArray(selectedValsLocal.value);

	if (!props.allowCustom) {
		vals = vals.filter((val) => presetVals.value.includes(val));
	}

	return vals;
});

const customVals = computed<string[]>(() => {
	return selectedVals.value.filter((val) => !presetVals.value.includes(val));
Repository: directus/directus
File: app/src/interfaces/tags/tags.vue (Vue)

Code before:
	{
		iconRight: 'local_offer',
		allowCustom: true,
	},
);

const emit = defineEmits(['input']);

const presetVals = computed<string[]>(() => {
	if (props.presets !== undefined) return processArray(props.presets);
	return [];
});

function isStringValue(val: unknown): val is string {
	return props.rawEditorEnabled === true && typeof val === 'string';
}

const isVariableMode = ref(isStringValue(props.value));

const selectedValsLocal = ref<string[]>(Array.isArray(props.value) ? processArray(props.value) : []);

watch(
	() => props.value,
	(newVal) => {
		isVariableMode.value = isStringValue(newVal);

		if (Array.isArray(newVal)) {
			selectedValsLocal.value = processArray(newVal);
		}

		if (newVal === null) selectedValsLocal.value = [];
	},
);

const selectedVals = computed<string[]>(() => {
	let vals = processArray(selectedValsLocal.value);

	if (!props.allowCustom) {
		vals = vals.filter((val) => presetVals.value.includes(val));
	}

	return vals;
});

const customVals = computed<string[]>(() => {
	return selectedVals.value.filter((val) => !presetVals.value.includes(val));
});

function processArray(array: string[]): string[] {
	array = array.map((val) => {
		val = val.trim();

Review comment (suggestion):
We can delete this since we use a computed — see above.

```suggestion
```

Code after:
	{
		iconRight: 'local_offer',
		allowCustom: true,
	},
);

const emit = defineEmits(['input']);

const presetVals = computed<string[]>(() => {
	if (props.presets !== undefined) return processArray(props.presets);
	return [];
});

// Variable mode: when `rawEditorEnabled` is true and the value is a string rather than an array.
// This happens in contexts like Flows and Insights where the field value can be a dynamic
// variable expression (e.g. "{{$trigger.body.tags}}") instead of an array of tags.
const isVariableMode = computed(() => props.rawEditorEnabled === true && typeof props.value === 'string');

const selectedValsLocal = ref<string[]>(Array.isArray(props.value) ? processArray(props.value) : []);

watch(
	() => props.value,
	(newVal) => {
		if (Array.isArray(newVal)) {
			selectedValsLocal.value = processArray(newVal);
		}

		if (newVal === null) selectedValsLocal.value = [];
	},
);

const selectedVals = computed<string[]>(() => {
	let vals = processArray(selectedValsLocal.value);

	if (!props.allowCustom) {
		vals = vals.filter((val) => presetVals.value.includes(val));
	}

	return vals;
});

const customVals = computed<string[]>(() => {
	return selectedVals.value.filter((val) => !presetVals.value.includes(val));
});

function processArray(array: string[]): string[] {
	array = array.map((val) => {
		val = val.trim();
		if (props.capitalization === 'uppercase') val = val.toUpperCase();
		if (props.capitalization === 'lowercase') val = val.toLowerCase();