ronantakizawa/codereview-bench huggingface dataset:
Repository: withastro/astro
File: packages/astro/test/fixtures/live-loaders/src/live.config.ts (TypeScript)

Code before:
};

interface CollectionFilter {
	addToAge?: number;
	returnInvalid?: boolean;
}

type EntryFilter = {
	id: keyof typeof entries;
	addToAge?: number;
};

const entries = {
	'123': { id: '123', data: { title: 'Page 123', age: 10 } },
	'456': { id: '456', data: { title: 'Page 456', age: 20 } },
	'789': { id: '789', data: { title: 'Page 789', age: 30 } },
};

class CustomError extends Error {
	constructor(message: string) {
		super(message);
		this.name = 'CustomError';
	}
}

export const loader: LiveLoader<Entry, EntryFilter, CollectionFilter, CustomError> = {
	name: 'test-loader',
	loadEntry: async ({ filter }) => {
		const entry = entries[filter.id];
		if (!entry) {
			return {
				error: new CustomError(`Entry ${filter.id} not found`),
			};
		}
		return {
			...entry,
			data: {
				title: entry.data.title,
				age: filter?.addToAge
					? entry.data.age
						? entry.data.age + filter.addToAge
						: filter.addToAge
					: entry.data.age,
			},
			cacheHint: {
				tags: [`page:${filter.id}`],
				maxAge: 60,
				lastModified: new Date('2025-01-01T00:00:00.000Z'),
			},
		};
	},

Review comment (suggestion):
```suggestion
const loader: LiveLoader<Entry, EntryFilter, CollectionFilter, CustomError> = {
```

Code after:
};

interface CollectionFilter {
	addToAge?: number;
	returnInvalid?: boolean;
}

type EntryFilter = {
	id: keyof typeof entries;
	addToAge?: number;
};

const entries = {
	'123': { id: '123', data: { title: 'Page 123', age: 10 } },
	'456': { id: '456', data: { title: 'Page 456', age: 20 } },
	'789': { id: '789', data: { title: 'Page 789', age: 30 } },
};

class CustomError extends Error {
	constructor(message: string) {
		super(message);
		this.name = 'CustomError';
	}
}

const loader: LiveLoader<Entry, EntryFilter, CollectionFilter, CustomError> = {
	name: 'test-loader',
	loadEntry: async ({ filter }) => {
		const entry = entries[filter.id];
		if (!entry) {
			return {
				error: new CustomError(`Entry ${filter.id} not found`),
			};
		}
		return {
			...entry,
			data: {
				title: entry.data.title,
				age: filter?.addToAge
					? entry.data.age
						? entry.data.age + filter.addToAge
						: filter.addToAge
					: entry.data.age,
			},
			cacheHint: {
				tags: [`page:${filter.id}`],
				maxAge: 60,
				lastModified: new Date('2025-01-01T00:00:00.000Z'),
			},
		};
	},
Repository: withastro/astro
File: packages/astro/src/core/encryption.ts (TypeScript)

Code before:
	const decryptedBuffer = await crypto.subtle.decrypt(
		{
			name: ALGORITHM,
			iv,
		},
		key,
		dataArray,
	);
	const decryptedString = decoder.decode(decryptedBuffer);
	return decryptedString;
}

/**
 * Generates an SHA-256 digest of the given string.
 * @param {string} data The string to hash.
 * @param {CspAlgorithm} algorithm The algorithm to use.
 */
export async function generateCspDigest(data: string, algorithm: CspAlgorithm): Promise<CspHash> {
	const hashBuffer = await crypto.subtle.digest(algorithm, encoder.encode(data));

	const hash = encodeBase64(new Uint8Array(hashBuffer));
	return `${ALGORITHMS[algorithm]}${hash}`;
}

/**
 * Generate SHA-256 hash of buffer content for deduplication
 * @param {ArrayBuffer} data The buffer data to hash
 * @returns {Promise<string>} A hex string of the first 16 characters of the SHA-256 hash
 */
export async function generateContentHash(data: ArrayBuffer): Promise<string> {
	const hashBuffer = await crypto.subtle.digest('SHA-256', data);
	const hashArray = new Uint8Array(hashBuffer);
	const hashHex = Array.from(hashArray)
		.map(b => b.toString(16).padStart(2, '0'))
		.join('');
	return hashHex.slice(0, 16);
}


Review comment (suggestion):
```suggestion
 * Generate SHA-256 hash of a buffer. 
```

The function doesn't have anything to do with "content duplication".

Code after:
	const decryptedBuffer = await crypto.subtle.decrypt(
		{
			name: ALGORITHM,
			iv,
		},
		key,
		dataArray,
	);
	const decryptedString = decoder.decode(decryptedBuffer);
	return decryptedString;
}

/**
 * Generates an SHA-256 digest of the given string.
 * @param {string} data The string to hash.
 * @param {CspAlgorithm} algorithm The algorithm to use.
 */
export async function generateCspDigest(data: string, algorithm: CspAlgorithm): Promise<CspHash> {
	const hashBuffer = await crypto.subtle.digest(algorithm, encoder.encode(data));

	const hash = encodeBase64(new Uint8Array(hashBuffer));
	return `${ALGORITHMS[algorithm]}${hash}`;
}

/**
 * Generate SHA-256 hash of buffer. 
 * @param {ArrayBuffer} data The buffer data to hash
 * @returns {Promise<string>} A hex string of the first 16 characters of the SHA-256 hash
 */
export async function generateContentHash(data: ArrayBuffer): Promise<string> {
	const hashBuffer = await crypto.subtle.digest('SHA-256', data);
	const hashArray = new Uint8Array(hashBuffer);
	return encodeBase64(hashArray);
}
Repository: withastro/astro
File: packages/integrations/cloudflare/src/index.ts (TypeScript)

Code before:
	let _config: AstroConfig;
	let finalBuildOutput: HookParameters<'astro:config:done'>['buildOutput'];

	const cloudflareModulePlugin: PluginOption & CloudflareModulePluginExtra = cloudflareModuleLoader(
		args?.cloudflareModules ?? true,
	);

	let _routes: IntegrationResolvedRoute[];

	return {
		name: '@astrojs/cloudflare',
		hooks: {
			'astro:config:setup': ({
				command,
				config,
				updateConfig,
				logger,
				addWatchFile,
				addMiddleware,
			}) => {
				let session = config.session;

				const isBuild = command === 'build';

				if (!session?.driver) {
					const bindingName = args?.sessionKVBindingName ?? 'SESSION';

					logger.info(
						`Enabling sessions with Cloudflare KV for ${isBuild ? 'production' : 'development'} with the "${bindingName}" KV binding.`,
					);
					logger.info(
						`If you see the error "Invalid binding \`${bindingName}\`" in your build output, you need to add the binding to your wrangler config file.`,
					);

					session = {
						...session,
						driver: 'cloudflare-kv-binding',
						options: {
							binding: bindingName,
							...session?.options,
						},
					};
				}

				updateConfig({
					build: {
						client: new URL(`.${wrapWithSlashes(config.base)}`, config.outDir),
						server: new URL('./_worker.js/', config.outDir),
						serverEntry: 'index.js',
						redirects: false,
					},

Review comment (suggestion):
You mentioned wanting a way to add a default for the KV binding name. You could do it by hoisting `const bindingName = args?.sessionKVBindingName ?? 'SESSION';` up to the top level of `createIntegration` where you can use it in all of the hooks

Code after:
	let _config: AstroConfig;
	let finalBuildOutput: HookParameters<'astro:config:done'>['buildOutput'];

	const cloudflareModulePlugin: PluginOption & CloudflareModulePluginExtra = cloudflareModuleLoader(
		args?.cloudflareModules ?? true,
	);

	let _routes: IntegrationResolvedRoute[];

	const SESSION_KV_BINDING_NAME = args?.sessionKVBindingName ?? 'SESSION';

	return {
		name: '@astrojs/cloudflare',
		hooks: {
			'astro:config:setup': ({
				command,
				config,
				updateConfig,
				logger,
				addWatchFile,
				addMiddleware,
			}) => {
				let session = config.session;


				if (!session?.driver) {
					logger.info(
						`Enabling sessions with Cloudflare KV with the "${SESSION_KV_BINDING_NAME}" KV binding.`,
					);
					logger.info(
						`If you see the error "Invalid binding \`${SESSION_KV_BINDING_NAME}\`" in your build output, you need to add the binding to your wrangler config file.`,
					);

					session = {
						...session,
						driver: 'cloudflare-kv-binding',
						options: {
							binding: SESSION_KV_BINDING_NAME,
							...session?.options,
						},
					};
				}

				updateConfig({
					build: {
						client: new URL(`.${wrapWithSlashes(config.base)}`, config.outDir),
						server: new URL('./_worker.js/', config.outDir),
						serverEntry: 'index.js',
						redirects: false,
					},
					session,
Repository: withastro/astro
File: packages/integrations/cloudflare/src/index.ts (TypeScript)

Code before:
	const cloudflareModulePlugin: PluginOption & CloudflareModulePluginExtra = cloudflareModuleLoader(
		args?.cloudflareModules ?? true,
	);

	let _routes: IntegrationResolvedRoute[];

	const SESSION_KV_BINDING_NAME = args?.sessionKVBindingName ?? 'SESSION';

	return {
		name: '@astrojs/cloudflare',
		hooks: {
			'astro:config:setup': ({
				command,
				config,
				updateConfig,
				logger,
				addWatchFile,
				addMiddleware,
			}) => {
				let session = config.session;

				const isBuild = command === 'build';

				if (!session?.driver) {
					logger.info(
						`Enabling sessions with Cloudflare KV for ${isBuild ? 'production' : 'development'} with the "${SESSION_KV_BINDING_NAME}" KV binding.`,
					);
					logger.info(
						`If you see the error "Invalid binding \`${SESSION_KV_BINDING_NAME}\`" in your build output, you need to add the binding to your wrangler config file.`,
					);

					session = {
						...session,
						driver: 'cloudflare-kv-binding',
						options: {
							binding: SESSION_KV_BINDING_NAME,
							...session?.options,
						},
					};
				}

				updateConfig({
					build: {
						client: new URL(`.${wrapWithSlashes(config.base)}`, config.outDir),
						server: new URL('./_worker.js/', config.outDir),
						serverEntry: 'index.js',
						redirects: false,
					},
					session,
					vite: {
						plugins: [

Review comment (suggestion):
```suggestion
						`Enabling sessions with Cloudflare KV with the "${SESSION_KV_BINDING_NAME}" KV binding.`,
```
I think that's redundant now. It only said "for production" before because it wasn't used in dev.

Code after:
	const cloudflareModulePlugin: PluginOption & CloudflareModulePluginExtra = cloudflareModuleLoader(
		args?.cloudflareModules ?? true,
	);

	let _routes: IntegrationResolvedRoute[];

	const SESSION_KV_BINDING_NAME = args?.sessionKVBindingName ?? 'SESSION';

	return {
		name: '@astrojs/cloudflare',
		hooks: {
			'astro:config:setup': ({
				command,
				config,
				updateConfig,
				logger,
				addWatchFile,
				addMiddleware,
			}) => {
				let session = config.session;


				if (!session?.driver) {
					logger.info(
						`Enabling sessions with Cloudflare KV with the "${SESSION_KV_BINDING_NAME}" KV binding.`,
					);
					logger.info(
						`If you see the error "Invalid binding \`${SESSION_KV_BINDING_NAME}\`" in your build output, you need to add the binding to your wrangler config file.`,
					);

					session = {
						...session,
						driver: 'cloudflare-kv-binding',
						options: {
							binding: SESSION_KV_BINDING_NAME,
							...session?.options,
						},
					};
				}

				updateConfig({
					build: {
						client: new URL(`.${wrapWithSlashes(config.base)}`, config.outDir),
						server: new URL('./_worker.js/', config.outDir),
						serverEntry: 'index.js',
						redirects: false,
					},
					session,
					vite: {
						plugins: [
							// https://developers.cloudflare.com/pages/functions/module-support/
Repository: withastro/astro
File: packages/astro/types.d.ts (TypeScript)

Code before:
import './astro-jsx';
import type { OmitIndexSignature, Simplify } from './dist/type-utils.js';
import type { AstroBuiltinAttributes } from './dist/types/public/elements.js';
import type { BundledLanguage, SpecialLanguage, LanguageRegistration } from 'shiki';

/** Any supported HTML or SVG element name, as defined by the HTML specification */
export type HTMLTag = keyof astroHTML.JSX.DefinedIntrinsicElements;

/** The built-in attributes for any known HTML or SVG element name */
export type HTMLAttributes<Tag extends HTMLTag> = Omit<
	astroHTML.JSX.DefinedIntrinsicElements[Tag],
	keyof Omit<AstroBuiltinAttributes, 'class:list'>
>;

/**
 * All the CSS properties available, as defined by the CSS specification
 */
export type CSSProperty = keyof astroHTML.JSX.KebabCSSDOMProperties;

type PolymorphicAttributes<P extends { as: HTMLTag }> = Omit<P, 'as'> & {
	as?: P['as'];
} & Omit<
		// This is the same as HTMLAttributes<P['as']>, except we're using OmitIndexSignature to remove the index signature,
		// used for data attribute, because it seems like it get too complex for TypeScript with it, not sure why.
		OmitIndexSignature<astroHTML.JSX.DefinedIntrinsicElements[P['as']]>,
		keyof Omit<AstroBuiltinAttributes, 'class:list'>
	>;

export type Polymorphic<P extends { as: HTMLTag }> = PolymorphicAttributes<

Review comment (refactor):
I think we need to move this to a file in `src/types/public` instead, I need to investigate

Code after:
import './astro-jsx';
import type { OmitIndexSignature, Simplify } from './dist/type-utils.js';
import type { AstroBuiltinAttributes } from './dist/types/public/elements.js';

/** Any supported HTML or SVG element name, as defined by the HTML specification */
export type HTMLTag = keyof astroHTML.JSX.DefinedIntrinsicElements;

/** The built-in attributes for any known HTML or SVG element name */
export type HTMLAttributes<Tag extends HTMLTag> = Omit<
	astroHTML.JSX.DefinedIntrinsicElements[Tag],
	keyof Omit<AstroBuiltinAttributes, 'class:list'>
>;

/**
 * All the CSS properties available, as defined by the CSS specification
 */
export type CSSProperty = keyof astroHTML.JSX.KebabCSSDOMProperties;

type PolymorphicAttributes<P extends { as: HTMLTag }> = Omit<P, 'as'> & {
	as?: P['as'];
} & Omit<
		// This is the same as HTMLAttributes<P['as']>, except we're using OmitIndexSignature to remove the index signature,
		// used for data attribute, because it seems like it get too complex for TypeScript with it, not sure why.
		OmitIndexSignature<astroHTML.JSX.DefinedIntrinsicElements[P['as']]>,
		keyof Omit<AstroBuiltinAttributes, 'class:list'>
	>;

export type Polymorphic<P extends { as: HTMLTag }> = PolymorphicAttributes<
	Omit<P, 'as'> & { as: NonNullable<P['as']> }
Repository: withastro/astro
File: packages/db/src/core/integration/vite-plugin-db.ts (TypeScript)

Code before:

	return `
import { asDrizzleTable, normalizeDatabaseUrl } from ${RUNTIME_IMPORT};

${clientImport}

const dbUrl = normalizeDatabaseUrl(${JSON.stringify(ASTRO_DATABASE_FILE)}, ${JSON.stringify(dbUrl)});
export const db = createClient({ url: dbUrl });

export * from ${RUNTIME_VIRTUAL_IMPORT};

${getStringifiedTableExports(tables)}`;
}

export function getRemoteVirtualModContents({
	tables,
	appToken,
	isBuild,
	output,
	__execute = false, // Used for execute command
}: {
	tables: DBTables;
	appToken: string;
	isBuild: boolean;
	output: AstroConfig['output'];
	__execute?: boolean;
}) {
	const dbInfo = getRemoteDatabaseInfo();

	function appTokenArg() {
		if (isBuild) {
			if (output === 'server') {
				// In production build, always read the runtime environment variable.
				return `process.env.ASTRO_DB_APP_TOKEN`;
			} else {
				// Static mode or prerendering needs the local app token.
				return `process.env.ASTRO_DB_APP_TOKEN ?? ${JSON.stringify(appToken)}`;
			}
		} else {
			return JSON.stringify(appToken);
		}
	}

	function dbUrlArg() {
		const dbStr = JSON.stringify(dbInfo.url);

		if (isBuild) {
			// Allow overriding, mostly for testing
			return `import.meta.env.ASTRO_DB_REMOTE_URL ?? ${dbStr}`;
		} else {
			return dbStr;

Review comment (suggestion):
```suggestion
  // Request module to be loaded immediately in process
	localExecution?: boolean;
```

Code after:
 *
 * @param localExecution - Whether the DB client is being used in a local execution context (e.g. CLI commands).
 * @returns The module import string for the DB client.
 */
function getDBModule(localExecution: boolean) {
	return localExecution
		? `import { createClient } from '${DB_CLIENTS.node}';`
		: `import { createClient } from '${VIRTUAL_CLIENT_MODULE_ID}';`;
}

export function getLocalVirtualModContents({
	tables,
	root,
	localExecution,
}: {
	tables: DBTables;
	root: URL;
	/**
	 * Used for the execute command to import the client directly.
	 * In other cases, we use the runtime only vite virtual module.
	 *
	 * This is used to ensure that the client is imported correctly
	 * when executing commands like `astro db execute`.
	 */
	localExecution: boolean;
}) {
	const { ASTRO_DATABASE_FILE } = getAstroEnv();
	const dbUrl = new URL(DB_PATH, root);

	// If this is for the execute command, we need to import the client directly instead of using the runtime only virtual module.
	const clientImport = getDBModule(localExecution);

	return `
import { asDrizzleTable, normalizeDatabaseUrl } from ${RUNTIME_IMPORT};

${clientImport}

const dbUrl = normalizeDatabaseUrl(${JSON.stringify(ASTRO_DATABASE_FILE)}, ${JSON.stringify(dbUrl)});
export const db = createClient({ url: dbUrl });

export * from ${RUNTIME_VIRTUAL_IMPORT};

${getStringifiedTableExports(tables)}`;
}

export function getRemoteVirtualModContents({
	tables,
	appToken,
	isBuild,
	output,
	localExecution,
Repository: withastro/astro
File: packages/db/src/core/integration/vite-plugin-db-client.ts (TypeScript)

Code before:
import { DB_CLIENTS, VIRTUAL_CLIENT_MODULE_ID } from '../consts.js';
import type { VitePlugin } from '../utils.js';

type VitePluginDBClientParams = {
	connectToRemote: boolean;
	mode: 'node' | 'web';
};

function getRemoteClientModule(mode: 'node' | 'web') {
	switch (mode) {
		case 'web': {
			return `export { createClient } from '${DB_CLIENTS.web}';`;
		}
        case 'node':
		default:
			return `export { createClient } from '${DB_CLIENTS.node}';`;
	}
}

function getLocalClientModule(mode: 'node' | 'web') {
	switch (mode) {
        case 'node':
        case 'web':
		default:
			return `export { createClient } from '${DB_CLIENTS.local}';`;
	}
}

const resolved = '\0' + VIRTUAL_CLIENT_MODULE_ID;

export function vitePluginDbClient(params: VitePluginDBClientParams): VitePlugin {
	return {
		name: 'virtual:astro:db-client',
		enforce: 'pre',
		async resolveId(id) {
			if (id !== VIRTUAL_CLIENT_MODULE_ID) return;
			return resolved;
		},

Review comment (suggestion):
```suggestion
		case 'web':
			return `export { createClient } from '${DB_CLIENTS.web}';`;
```

Code after:
import { DB_CLIENTS, VIRTUAL_CLIENT_MODULE_ID } from '../consts.js';
import type { VitePlugin } from '../utils.js';

type VitePluginDBClientParams = {
	connectToRemote: boolean;
	mode: 'node' | 'web';
};

function getRemoteClientModule(mode: 'node' | 'web') {
	switch (mode) {
		case 'web':
			return `export { createClient } from '${DB_CLIENTS.web}';`;
        case 'node':
		default:
			return `export { createClient } from '${DB_CLIENTS.node}';`;
	}
}

function getLocalClientModule(mode: 'node' | 'web') {
	switch (mode) {
        case 'node':
        case 'web':
		default:
			return `export { createClient } from '${DB_CLIENTS.local}';`;
	}
}

const resolved = '\0' + VIRTUAL_CLIENT_MODULE_ID;

export function vitePluginDbClient(params: VitePluginDBClientParams): VitePlugin {
	return {
		name: 'virtual:astro:db-client',
		enforce: 'pre',
		async resolveId(id) {
			if (id !== VIRTUAL_CLIENT_MODULE_ID) return;
			return resolved;
		},
		async load(id) {
Repository: withastro/astro
File: packages/db/src/core/integration/vite-plugin-db.ts (TypeScript)

Code before:
				// In production build, always read the runtime environment variable.
				return `process.env.ASTRO_DB_APP_TOKEN`;
			} else {
				// Static mode or prerendering needs the local app token.
				return `process.env.ASTRO_DB_APP_TOKEN ?? ${JSON.stringify(appToken)}`;
			}
		} else {
			return JSON.stringify(appToken);
		}
	}

	function dbUrlArg() {
		const dbStr = JSON.stringify(dbInfo.url);

		if (isBuild) {
			// Allow overriding, mostly for testing
			return `import.meta.env.ASTRO_DB_REMOTE_URL ?? ${dbStr}`;
		} else {
			return dbStr;
		}
	}

	// If this is for the execute command, we need to import the client directly instead of using the runtime only virtual module.
	const clientImport = localExecution
		? `import { createClient } from '${DB_CLIENTS.node}';`
		: `import { createClient } from '${VIRTUAL_CLIENT_MODULE_ID}';`;

	return `
import {asDrizzleTable} from ${RUNTIME_IMPORT};

${clientImport}

export const db = await createClient({
  url: ${dbUrlArg()},
  token: ${appTokenArg()},
});

export * from ${RUNTIME_VIRTUAL_IMPORT};

${getStringifiedTableExports(tables)}
	`;
}

function getStringifiedTableExports(tables: DBTables) {
	return Object.entries(tables)
		.map(
			([name, table]) =>
				`export const ${name} = asDrizzleTable(${JSON.stringify(name)}, ${JSON.stringify(
					table,
				)}, false)`,
		)

Review comment (suggestion):
To me, using a node client for a remote feels like an error. However, I am sure you think it's not. Is it because the execution of commands can be done only locally? If so, then I have two suggestions:
- commands shouldn't call `getRemoteVirtualModContents`
- add some JsDoc to `getRemoteVirtualModContents` that explains the implications of `localExecution` for this function 

Code after:

${clientImport}

const dbUrl = normalizeDatabaseUrl(${JSON.stringify(ASTRO_DATABASE_FILE)}, ${JSON.stringify(dbUrl)});
export const db = createClient({ url: dbUrl });

export * from ${RUNTIME_VIRTUAL_IMPORT};

${getStringifiedTableExports(tables)}`;
}

export function getRemoteVirtualModContents({
	tables,
	appToken,
	isBuild,
	output,
	localExecution,
}: {
	tables: DBTables;
	appToken: string;
	isBuild: boolean;
	output: AstroConfig['output'];
	/**
	 * Used for the execute command to import the client directly.
	 * In other cases, we use the runtime only vite virtual module.
	 *
	 * This is used to ensure that the client is imported correctly
	 * when executing commands like `astro db execute`.
	 */
	localExecution: boolean;
}) {
	const dbInfo = getRemoteDatabaseInfo();

	function appTokenArg() {
		if (isBuild) {
			if (output === 'server') {
				// In production build, always read the runtime environment variable.
				return `process.env.ASTRO_DB_APP_TOKEN`;
			} else {
				// Static mode or prerendering needs the local app token.
				return `process.env.ASTRO_DB_APP_TOKEN ?? ${JSON.stringify(appToken)}`;
			}
		} else {
			return JSON.stringify(appToken);
		}
	}

	function dbUrlArg() {
		const dbStr = JSON.stringify(dbInfo.url);

		if (isBuild) {