ronantakizawa/codereview-bench huggingface dataset:
Repository: wandb/openui
File: frontend/src/api/openai.ts (TypeScript)

Code before:
					text: 'This is a screenshot of a web component I want to replicate.  Please generate HTML for it.'
				},
				{
					type: 'image_url',
					image_url: {
						url: imageUrl
					}
				}
			]
		})
	}

	if (html === undefined || html === '') {
		if (query !== 'image-upload') {
			messages.push({
				role: 'user',
				content: query
			})
		}
	} else {
		let userPrompt = 'Address the FIX comments.'
		if (query !== 'html-comments-only') {
			userPrompt = query
		}
		const instructions = `Given the following HTML:\n\n${html}\n\n${userPrompt}`
		console.log('Providing instructions:\n', instructions)
		messages.push({
			role: 'user',
			content: instructions
		})
	}

	const response = await openai.chat.completions.create({
		model, // can change to "gpt-4" if you fancy
		messages,
		temperature,
		stream: true,
		max_tokens: GPT4_MAX_TOKENS
	})
	let markdown = ''
	for await (const chunk of response) {
		const part = chunk.choices[0]?.delta?.content ?? ''
		markdown += part
		callback(part)
	}
	return markdown
}

const systemPromptConvert = `You're a frontend web developer that specializes in $FRAMEWORK.
Given html and javascript, generate a $FRAMEWORK component. Factor the code into smaller
components if necessary. Keep all code in one file. Use hooks and put tailwind class strings

Review comment (suggestion):
I think this logic might be problematic.  Imagine the following scenario:

1. User generates HTML
2. User annotates with a fix comment
3. User wants to iterate again with a query comment

The HTML that's coming in at #3 above might still have <!--FIX comments in it.  I've definitely seen the LLM return new HTML that still has the FIX comments.  I know I have logic somewhere to just remove comments from the HTML which we could do on ingest and this might be ok...  I'll give it a try locally and see if it's a problem.

Code after:
				: '';
			messages.push({
				role: 'user',
				content: [
					{
						type: 'text',
						text: `This is a screenshot of a web component I want to replicate.  Please generate HTML for it.\n ${textImageRequirements}`
					},
					{
						type: 'image_url',
						image_url: {
							url: imageUrl
						}
					}
				]
			})
		} else { 
			messages.push({
				role: 'user',
				content: query
			})
		}
	} else {
		// Annotation comments should like <!--FIX (1): make the image larger-->
		const hasAnnotationComments = /<!--FIX (\(\d+\)): (.+)-->/g.test(html as string);
		let userPrompt = hasAnnotationComments ? 'Address the FIX comments.': query
		
		const instructions = `Given the following HTML:\n\n${html}\n\n${userPrompt}`
		console.log('Providing instructions:\n', instructions)
		messages.push({
			role: 'user',
			content: instructions
		})
	}

	const response = await openai.chat.completions.create({
		model, // can change to "gpt-4" if you fancy
		messages,
		temperature,
		stream: true,
		max_tokens: GPT4_MAX_TOKENS
	})
	let markdown = ''
	for await (const chunk of response) {
		const part = chunk.choices[0]?.delta?.content ?? ''
		markdown += part
		callback(part)
	}
	return markdown
}
Repository: xenova/transformers.js
File: src/generation.js (JavaScript)

Code before:
  } else {
    const generatedNgrams = getNgrams(ngramSize, prevInputIds, numHypos);

    for (let i = 0; i < numHypos; ++i) {
      const hypoPrevInputIds = prevInputIds.slice(i * curLen, (i + 1) * curLen);
      const bannedTokens = getGeneratedNgrams(
        generatedNgrams[i],
        hypoPrevInputIds,
        ngramSize,
        curLen
      );
      batchBannedTokens.push(bannedTokens);
    }
  }
  return batchBannedTokens;
};

class NoRepeatNGramLogitsProcessor extends LogitsProcessor {
  constructor(no_repeat_ngram_size) {
    super();
    this.no_repeat_ngram_size = no_repeat_ngram_size;
  }

  _call(input_ids, logits) {
    const numBatchHypotheses = logits.dims[0];
    const curLen = input_ids.length / numBatchHypotheses;
    const bannedBatchTokens = calcBannedNgramTokens(
      this.no_repeat_ngram_size,
      input_ids,
      numBatchHypotheses,
      curLen
    );
    for (const [hypo, bannedTokens] of bannedBatchTokens.entries()) {
      for (const token of bannedTokens) {
        console.log("banned token", token);
        logits.data[hypo * logits.dims[1] + token] = -Infinity;``
      }
    }
    return logits;
  }
}

class GenerationConfig {
    constructor(kwargs = {}) {
        // Parameters that control the length of the output
        this.max_length = kwargs.max_length ?? 20;
        this.max_new_tokens = kwargs.max_new_tokens ?? null;
        this.min_length = kwargs.min_length ?? 0;
        this.min_new_tokens = kwargs.min_new_tokens ?? null;
        this.early_stopping = kwargs.early_stopping ?? false;
        this.max_time = kwargs.max_time ?? null;

Review comment (suggestion):
`curLen` is just `input_ids.length` (see comment on main thread for why)

Code after:
  } else {
    const generatedNgrams = getNgrams(ngramSize, prevInputIds, numHypos);

    for (let i = 0; i < numHypos; ++i) {
      const hypoPrevInputIds = prevInputIds.slice(i * curLen, (i + 1) * curLen);
      const bannedTokens = getGeneratedNgrams(
        generatedNgrams[i],
        hypoPrevInputIds,
        ngramSize,
        curLen
      );
      batchBannedTokens.push(bannedTokens);
    }
  }
  return batchBannedTokens;
};

class NoRepeatNGramLogitsProcessor extends LogitsProcessor {
  constructor(no_repeat_ngram_size) {
    super();
    this.no_repeat_ngram_size = no_repeat_ngram_size;
  }

  _call(input_ids, logits) {
    const numBatchHypotheses = logits.dims[0];
    const curLen = input_ids.length / numBatchHypotheses;
    const bannedBatchTokens = calcBannedNgramTokens(
      this.no_repeat_ngram_size,
      input_ids,
      numBatchHypotheses,
      curLen
    );
    for (const [hypo, bannedTokens] of bannedBatchTokens.entries()) {
      for (const token of bannedTokens) {
        logits.data[hypo * logits.dims[1] + token] = -Infinity;``
      }
    }
    return logits;
  }
}


class RepetitionPenaltyLogitsProcessor extends LogitsProcessor {
  constructor(penalty) {
    super();
    this.penalty = penalty;
  }

  _call(input_ids, logits) {
      const numBatches = logits.dims[0]
      const batchSize = logits.dims[1]
Repository: xenova/transformers.js
File: tests/tokenizers.test.js (JavaScript)

Code before:


import { AutoTokenizer } from '../src/transformers.js';
import { getFile } from '../src/utils/hub.js';
import { m } from './init.js';

// Load test data generated by the python tests
// TODO do this dynamically?
let testsData = await (await getFile('./tests/data/tokenizer_tests.json')).json()

describe('Tokenizers', () => {

    for (let [tokenizerName, tests] of Object.entries(testsData)) {

        it(tokenizerName, async () => {
            let tokenizer = await AutoTokenizer.from_pretrained(m(tokenizerName));

            for (let test of tests) {

                // Test encoding
                let encoded = tokenizer(test.input, {
                    return_tensor: false
                });
                expect(encoded).toEqual(test.encoded);

                // Test decoding
                let decoded_with_special = tokenizer.decode(encoded.input_ids, { skip_special_tokens: false });
                expect(decoded_with_special).toEqual(test.decoded_with_special);

                let decoded_without_special = tokenizer.decode(encoded.input_ids, { skip_special_tokens: true });
                expect(decoded_without_special).toEqual(test.decoded_without_special);
            }
        });
    }
});


Review comment (suggestion):
`'Hello world! '.repeat(50000);` :pinched_fingers: 

Kinda sad to see the nice array spreading syntax gone, but this is the reality of large arrays, nice work :1st_place_medal: 

Code after:
            let tokenizer = await AutoTokenizer.from_pretrained(m(tokenizerName));

            for (let test of tests) {

                // Test encoding
                let encoded = tokenizer(test.input, {
                    return_tensor: false
                });
                expect(encoded).toEqual(test.encoded);

                // Test decoding
                let decoded_with_special = tokenizer.decode(encoded.input_ids, { skip_special_tokens: false });
                expect(decoded_with_special).toEqual(test.decoded_with_special);

                let decoded_without_special = tokenizer.decode(encoded.input_ids, { skip_special_tokens: true });
                expect(decoded_without_special).toEqual(test.decoded_without_special);
            }
        });
    }
});

describe('Edge cases', () => {
    it('should not crash when encoding a very long string', async () => {
        let tokenizer = await AutoTokenizer.from_pretrained('t5-small');

        let text = String.prototype.repeat.call('Hello world! ', 50000);
        let encoded = await tokenizer(text);
        expect(encoded.input_ids.data.length).toBeGreaterThan(100000);
    });
});
Repository: xenova/transformers.js
File: src/tokenizers.js (JavaScript)

Code before:
     */
    constructor(config) {
        super(config);
        /**
         * A mapping of tokens to ids.
         * @type {Map<string, number>}
         */
        this.tokens_to_ids = objectToMap(config.vocab);

        /**
         * The id of the unknown token.
         * @type {number}
         */
        this.unk_token_id = this.tokens_to_ids.get(config.unk_token);

        /**
         * The unknown token string.
         * @type {string}
         */
        this.unk_token = config.unk_token;

        /**
         * The maximum number of characters allowed per word.
         * @type {number}
         */
        this.maxInputCharsPerWord = config.maxInputCharsPerWord || 100;

        /**
         * An array of tokens.
         * @type {string[]}
         */
        this.vocab = new Array(this.tokens_to_ids.size);
        for (const [key, value] of this.tokens_to_ids) {
            this.vocab[value] = key;
        }
    }

    /**
     * Encodes an array of tokens using WordPiece encoding.
     * @param {string[]} tokens The tokens to encode.
     * @returns {string[]} An array of encoded tokens.
     */
    encode(tokens) {
        let outputTokens = [];
        for (let token of tokens) {
            let chars = [...token];
            if (chars.length > this.maxInputCharsPerWord) {
                outputTokens.push(this.unk_token);
                continue;
            }


Review comment (suggestion):
```suggestion
        this.max_input_chars_per_word = config.max_input_chars_per_word ?? 100;
```

Code after:
     */
    constructor(config) {
        super(config);
        /**
         * A mapping of tokens to ids.
         * @type {Map<string, number>}
         */
        this.tokens_to_ids = objectToMap(config.vocab);

        /**
         * The id of the unknown token.
         * @type {number}
         */
        this.unk_token_id = this.tokens_to_ids.get(config.unk_token);

        /**
         * The unknown token string.
         * @type {string}
         */
        this.unk_token = config.unk_token;

        /**
         * The maximum number of characters allowed per word.
         * @type {number}
         */
        this.max_input_chars_per_word = config.max_input_chars_per_word ?? 100;

        /**
         * An array of tokens.
         * @type {string[]}
         */
        this.vocab = new Array(this.tokens_to_ids.size);
        for (const [key, value] of this.tokens_to_ids) {
            this.vocab[value] = key;
        }
    }

    /**
     * Encodes an array of tokens using WordPiece encoding.
     * @param {string[]} tokens The tokens to encode.
     * @returns {string[]} An array of encoded tokens.
     */
    encode(tokens) {
        let outputTokens = [];
        for (let token of tokens) {
            let chars = [...token];
            if (chars.length > this.max_input_chars_per_word) {
                outputTokens.push(this.unk_token);
                continue;
            }
Repository: xenova/transformers.js
File: src/utils/tensor.js (JavaScript)

Code before:
/**
 * @file Helper module for `Tensor` processing.
 * 
 * These functions and classes are only used internally, 
 * meaning an end-user shouldn't need to access anything here.
 * 
 * @module utils/tensor
 */

import { ONNX } from '../backends/onnx.js';

import {
    interpolate_data,
    transpose_data
} from './maths.js';


/**
 * @typedef {'bool'|'float32'|'float64'|'string'|'int8'|'uint8'|'int16'|'uint16'|'int32'|'uint32'|'int64'} DataType 
 */
const DataTypeMap = new Map( /** @type {[DataType, any][]} */([
    ['bool', Uint8Array],
    ['float32', Float32Array],
    ['float64', Float64Array],
    ['string', Array], // string[]
    ['int8', Int8Array],
    ['uint8', Uint8Array],
    ['int16', Int16Array],
    ['uint16', Uint16Array],
    ['int32', Int32Array],
    ['uint32', Uint32Array],
    ['int64', BigInt64Array],
]))

/**
 * @typedef {import('./maths.js').AnyTypedArray | any[]} DataArray
 */

const ONNXTensor = ONNX.Tensor;

export class Tensor {
    /** @type {DataType} Type of the tensor. */
    type;

    /** @type {DataArray} The data stored in the tensor. */
    data;

Review comment (suggestion):
This is creating so much ASCII soup, why not just:

```js
const DataTypeMap = {
    bool: Uint8Array,
    float32: Float32Array,
    float64: Float64Array,
    string: Array, // string[]
    int8: Int8Array,
    uint8: Uint8Array,
    int16: Int16Array,
    uint16: Uint16Array,
    int32: Int32Array,
    uint32: Uint32Array,
    int64: BigInt64Array,
};
```

You get better typing for free without any casting hacks:

![image](https://github.com/xenova/transformers.js/assets/5236548/a307088d-f9aa-4190-bb15-4bbbb103248d)

Probably just went overboard with "Map is new holy grail" suggestions? We don't need any of that, the order doesn't matter, it won't contain keys like `toString`, we only need strings as key etc.

Code after:
/**
 * @file Helper module for `Tensor` processing.
 * 
 * These functions and classes are only used internally, 
 * meaning an end-user shouldn't need to access anything here.
 * 
 * @module utils/tensor
 */

import { ONNX } from '../backends/onnx.js';

import {
    interpolate_data,
    transpose_data
} from './maths.js';


const DataTypeMap = Object.freeze({
    float32: Float32Array,
    float64: Float64Array,
    string: Array, // string[]
    int8: Int8Array,
    uint8: Uint8Array,
    int16: Int16Array,
    uint16: Uint16Array,
    int32: Int32Array,
    uint32: Uint32Array,
    int64: BigInt64Array,
    uint64: BigUint64Array,
    bool: Uint8Array,
});

/**
 * @typedef {keyof typeof DataTypeMap} DataType
 * @typedef {import('./maths.js').AnyTypedArray | any[]} DataArray
 */

const ONNXTensor = ONNX.Tensor;

export class Tensor {
    /** @type {number[]} Dimensions of the tensor. */
    dims;

    /** @type {DataType} Type of the tensor. */
    type;
Repository: xenova/transformers.js
File: src/utils/tensor.js (JavaScript)

Code before:
    interpolate_data,
    transpose_data
} from './maths.js';


/**
 * @typedef {'bool'|'float32'|'float64'|'string'|'int8'|'uint8'|'int16'|'uint16'|'int32'|'uint32'|'int64'} DataType 
 */
const DataTypeMap = new Map( /** @type {[DataType, any][]} */([
    ['bool', Uint8Array],
    ['float32', Float32Array],
    ['float64', Float64Array],
    ['string', Array], // string[]
    ['int8', Int8Array],
    ['uint8', Uint8Array],
    ['int16', Int16Array],
    ['uint16', Uint16Array],
    ['int32', Int32Array],
    ['uint32', Uint32Array],
    ['int64', BigInt64Array],
]))

/**
 * @typedef {import('./maths.js').AnyTypedArray | any[]} DataArray
 */

const ONNXTensor = ONNX.Tensor;

export class Tensor {
    /** @type {DataType} Type of the tensor. */
    type;

    /** @type {DataArray} The data stored in the tensor. */
    data;

    /** @type {number[]} Dimensions of the tensor. */
    dims;

    /**
     * Create a new Tensor or copy an existing Tensor.
     * @param {[DataType, DataArray, number[]]|[ONNXTensor]} args
     */
    constructor(...args) {
        if (args[0] instanceof ONNX.Tensor) {
            // Create shallow copy
            Object.assign(this, args[0]);

        } else {
            // Create new tensor
            Object.assign(this, new ONNX.Tensor(
                /** @type {DataType} */(args[0]),

Review comment (suggestion):
Only used as type for the constructor, but stripped from type definitions, so it will be unresolved/any, we can just remove the line

Code after:
    interpolate_data,
    transpose_data
} from './maths.js';


const DataTypeMap = Object.freeze({
    float32: Float32Array,
    float64: Float64Array,
    string: Array, // string[]
    int8: Int8Array,
    uint8: Uint8Array,
    int16: Int16Array,
    uint16: Uint16Array,
    int32: Int32Array,
    uint32: Uint32Array,
    int64: BigInt64Array,
    uint64: BigUint64Array,
    bool: Uint8Array,
});

/**
 * @typedef {keyof typeof DataTypeMap} DataType
 * @typedef {import('./maths.js').AnyTypedArray | any[]} DataArray
 */

const ONNXTensor = ONNX.Tensor;

export class Tensor {
    /** @type {number[]} Dimensions of the tensor. */
    dims;

    /** @type {DataType} Type of the tensor. */
    type;

    /** @type {DataArray} The data stored in the tensor. */
    data;

    /** @type {number} The number of elements in the tensor. */
    size;

    /**
     * Create a new Tensor or copy an existing Tensor.
     * @param {[DataType, DataArray, number[]]|[import('onnxruntime-common').Tensor]} args
     */
    constructor(...args) {
        if (args[0] instanceof ONNXTensor) {
            // Create shallow copy
            Object.assign(this, args[0]);

        } else {
            // Create new tensor
Repository: xenova/transformers.js
File: src/utils/tensor.js (JavaScript)

Code before:
    ['int16', Int16Array],
    ['uint16', Uint16Array],
    ['int32', Int32Array],
    ['uint32', Uint32Array],
    ['int64', BigInt64Array],
]))

/**
 * @typedef {import('./maths.js').AnyTypedArray | any[]} DataArray
 */

const ONNXTensor = ONNX.Tensor;

export class Tensor {
    /** @type {DataType} Type of the tensor. */
    type;

    /** @type {DataArray} The data stored in the tensor. */
    data;

    /** @type {number[]} Dimensions of the tensor. */
    dims;

    /**
     * Create a new Tensor or copy an existing Tensor.
     * @param {[DataType, DataArray, number[]]|[ONNXTensor]} args
     */
    constructor(...args) {
        if (args[0] instanceof ONNX.Tensor) {
            // Create shallow copy
            Object.assign(this, args[0]);

        } else {
            // Create new tensor
            Object.assign(this, new ONNX.Tensor(
                /** @type {DataType} */(args[0]),
                /** @type {Exclude<import('./maths.js').AnyTypedArray, Uint8ClampedArray>} */(args[1]),
                args[2]
            ));
        }

        return new Proxy(this, {
            get: (obj, key) => {
                if (typeof key === 'string') {
                    let index = Number(key);
                    if (Number.isInteger(index)) {
                        // key is an integer (i.e., index)
                        return obj._getitem(index);
                    }
                }
                // @ts-ignore

Review comment (suggestion):
```suggestion
     * @param {[DataType, DataArray, number[]]|[import('onnxruntime-common').Tensor]} args
```

Code after:
    int64: BigInt64Array,
    uint64: BigUint64Array,
    bool: Uint8Array,
});

/**
 * @typedef {keyof typeof DataTypeMap} DataType
 * @typedef {import('./maths.js').AnyTypedArray | any[]} DataArray
 */

const ONNXTensor = ONNX.Tensor;

export class Tensor {
    /** @type {number[]} Dimensions of the tensor. */
    dims;

    /** @type {DataType} Type of the tensor. */
    type;

    /** @type {DataArray} The data stored in the tensor. */
    data;

    /** @type {number} The number of elements in the tensor. */
    size;

    /**
     * Create a new Tensor or copy an existing Tensor.
     * @param {[DataType, DataArray, number[]]|[import('onnxruntime-common').Tensor]} args
     */
    constructor(...args) {
        if (args[0] instanceof ONNXTensor) {
            // Create shallow copy
            Object.assign(this, args[0]);

        } else {
            // Create new tensor
            Object.assign(this, new ONNXTensor(
                /** @type {DataType} */(args[0]),
                /** @type {Exclude<import('./maths.js').AnyTypedArray, Uint8ClampedArray>} */(args[1]),
                args[2]
            ));
        }

        return new Proxy(this, {
            get: (obj, key) => {
                if (typeof key === 'string') {
                    let index = Number(key);
                    if (Number.isInteger(index)) {
                        // key is an integer (i.e., index)
                        return obj._getitem(index);
                    }
Repository: xenova/transformers.js
File: scripts/convert.py (Python)

Code before:
        'reduce_range': False,
    },
    'opt': {
        'per_channel': False,
        'reduce_range': False,
    },
    'mistral': {
        'per_channel': False,
        'reduce_range': False,
    },
    'falcon': {
        'per_channel': False,
        'reduce_range': False,
    },
    'phi': {
        'per_channel': False,
        'reduce_range': False,
    },
    'qwen2': {
        'per_channel': False,
        'reduce_range': False,
    },
    'stablelm': {
      'per_channel': False,
        'reduce_range': False,
    },
    'starcoder2': {
        'per_channel': False,
        'reduce_range': False,
    },

    # Encoder-decoder models
    'whisper': {
        'per_channel': False,
        'reduce_range': False,
    },
    'vision-encoder-decoder': {
        'per_channel': False,
        'reduce_range': False,
    },

    # Encoder-only models
    'owlv2': {
        'per_channel': False,
        'reduce_range': False,
    },
    'wavlm': {
        'per_channel': False,
        'reduce_range': False,
    },
    'wav2vec2': {

Review comment (suggestion):
```suggestion
    'stablelm': {
        'per_channel': False,
        'reduce_range': False,
    },
```

Code after:
        'reduce_range': False,
    },
    'opt': {
        'per_channel': False,
        'reduce_range': False,
    },
    'mistral': {
        'per_channel': False,
        'reduce_range': False,
    },
    'falcon': {
        'per_channel': False,
        'reduce_range': False,
    },
    'phi': {
        'per_channel': False,
        'reduce_range': False,
    },
    'qwen2': {
        'per_channel': False,
        'reduce_range': False,
    },
    'stablelm': {
        'per_channel': False,
        'reduce_range': False,
    },
    'starcoder2': {
        'per_channel': False,
        'reduce_range': False,
    },

    # Encoder-decoder models
    'whisper': {
        'per_channel': False,
        'reduce_range': False,
    },
    'vision-encoder-decoder': {
        'per_channel': False,
        'reduce_range': False,
    },

    # Encoder-only models
    'owlv2': {
        'per_channel': False,
        'reduce_range': False,
    },
    'wavlm': {
        'per_channel': False,
        'reduce_range': False,
    },
    'wav2vec2': {