FEATURE: Update Testbench ui/ux #650
Conversation
WalkthroughStore-driven TestBenchCreator modal replaces local dialog state; title moved into body; UI restructured around group-centric data (groups/tests/inputs/outputs/bitwidths); new add/delete/reset/import/export flows; CSV format changed to group-driven G-/I-/O- blocks; Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (2)
7-7: Add accessible labels to icon-only buttons.Improves a11y for screen readers.
-<v-btn icon size="small" variant="text" @click="testBenchStore.showTestBenchCreator = false" class="close-button"> +<v-btn icon size="small" variant="text" aria-label="Close" @click="testBenchStore.showTestBenchCreator = false" class="close-button">-<v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" @click="increInputs"><v-icon>mdi-plus</v-icon></v-btn> +<v-btn icon size="x-small" variant="flat" aria-label="Add input" class="ml-2 action-icon" @click="increInputs"><v-icon>mdi-plus</v-icon></v-btn>-<v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" @click="increOutputs"><v-icon>mdi-plus</v-icon></v-btn> +<v-btn icon size="x-small" variant="flat" aria-label="Add output" class="ml-2 action-icon" @click="increOutputs"><v-icon>mdi-plus</v-icon></v-btn>-<v-btn icon size="x-small" variant="text" class="delete-io-btn" @click="deleteInput(i)"><v-icon size="small">mdi-minus-circle-outline</v-icon></v-btn> +<v-btn icon size="x-small" variant="text" aria-label="Remove input" class="delete-io-btn" @click="deleteInput(i)"><v-icon size="small">mdi-minus-circle-outline</v-icon></v-btn>-<v-btn icon size="x-small" variant="text" class="delete-io-btn" @click="deleteOutput(i)"><v-icon size="small">mdi-minus-circle-outline</v-icon></v-btn> +<v-btn icon size="x-small" variant="text" aria-label="Remove output" class="delete-io-btn" @click="deleteOutput(i)"><v-icon size="small">mdi-minus-circle-outline</v-icon></v-btn>-<v-btn icon size="x-small" variant="text" class="delete-io-btn" @click="deleteTestFromGroup(groupIndex, testIndex)"><v-icon size="small">mdi-close</v-icon></v-btn> +<v-btn icon size="x-small" variant="text" aria-label="Remove test row" class="delete-io-btn" @click="deleteTestFromGroup(groupIndex, testIndex)"><v-icon size="small">mdi-close</v-icon></v-btn>Also applies to: 49-49, 52-52, 65-65, 71-71, 103-103
159-159: Type-safety for results: prefer ref over reactive([]) for arrays.This avoids TS pitfalls with reactive arrays and keeps updates explicit.
-const results: boolean[][][] = reactive([]); +const results = ref<boolean[][][]>([]);- results.splice(0, results.length); + results.value.splice(0, results.value.length); @@ - results.push(groupResults); + results.value.push(groupResults);Template usage remains the same due to ref unwrapping.
Also applies to: 225-241, 115-119
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)
🔇 Additional comments (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
95-98: Vuetify API check: v-icon “left” is v2-only.If on Vuetify 3, use prepend-icon on v-btn instead.
-<v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)"> - <v-icon left>mdi-plus</v-icon> - Add Test -</v-btn> +<v-btn size="small" class="add-test-btn" variant="tonal" prepend-icon="mdi-plus" @click="addTestToGroup(groupIndex)"> + Add Test +</v-btn>
|
@aryann PTAL, Next I ll raise PR for TesbenchValidator |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (2)
300-313: Allow adding the first Input column even when no columns exist.Early-return blocks adding the first column. Populate an empty column with current test count.
-const increInputs = () => { - groups.forEach((group) => { - if (group.inputs.length === 0) return; - - group.inputs.push([]); - - for (let i = 0; i < group.inputs[0].length; i++) { - group.inputs[group.inputs.length - 1].push("0"); - } - }); - - inputsBandWidth.value.push(1); - inputsName.value.push(`inp${inputsName.value.length + 1}`); -}; +const increInputs = () => { + groups.forEach((group) => { + const testCount = group.inputs[0]?.length ?? 0; + const newCol: string[] = Array.from({ length: testCount }, () => "0"); + group.inputs.push(newCol); + }); + inputsBandWidth.value.push(1); + inputsName.value.push(`inp${inputsName.value.length + 1}`); +};
327-340: Allow adding the first Output column even when no columns exist.Mirror the Inputs fix for Outputs.
-const increOutputs = () => { - groups.forEach((group) => { - if (group.outputs.length === 0) return; - - group.outputs.push([]); - - for (let i = 0; i < group.outputs[0].length; i++) { - group.outputs[group.outputs.length - 1].push("0"); - } - }); - - outputsBandWidth.value.push(1); - outputsName.value.push(`out${outputsName.value.length + 1}`); -}; +const increOutputs = () => { + groups.forEach((group) => { + const testCount = group.outputs[0]?.length ?? 0; + const newCol: string[] = Array.from({ length: testCount }, () => "0"); + group.outputs.push(newCol); + }); + outputsBandWidth.value.push(1); + outputsName.value.push(`out${outputsName.value.length + 1}`); +};
♻️ Duplicate comments (5)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (5)
82-89: Make Bitwidth inputs numeric and bounded to avoid invalid maxlength and later width issues.Switch to number inputs with .number modifier and bounds.
- <input class="io-input bitwidth-input" type="text" v-model="inputsBandWidth[i]" maxlength="1" /> + <input class="io-input bitwidth-input" type="number" v-model.number="inputsBandWidth[i]" min="1" max="64" />- <input class="io-input bitwidth-input" type="text" v-model="outputsBandWidth[i]" maxlength="1" /> + <input class="io-input bitwidth-input" type="number" v-model.number="outputsBandWidth[i]" min="1" max="64" />
166-173: Seed group columns to prevent undefined iteration and make “+ Inputs/Outputs” work on empty groups.Current groups start with empty inputs/outputs, so
group.inputs[0]can be undefined and header “+” buttons no-op on empty groups. Pre-create empty columns based on current IO names.-const groups = reactive<Group[]>([ - { - title: 'Group 1', - inputs: [], - outputs: [], - } -]); +const groups = reactive<Group[]>([ + createEmptyGroup('Group 1') +]);-const addNewGroup = () => { - groups.push({ - title: `Group ${groups.length + 1}`, - inputs: [], - outputs: [], - }); -}; +const addNewGroup = () => { + groups.push(createEmptyGroup(`Group ${groups.length + 1}`)); +};const resetData = () => { testTitle.value = 'Untitled'; testType.value = 'comb'; inputsBandWidth.value = [1]; outputsBandWidth.value = [1]; inputsName.value = ["inp1"]; outputsName.value = ["out1"]; - groups.splice(0, groups.length, { - title: 'Group 1', - inputs: [], - outputs: [], - }); + groups.splice(0, groups.length, createEmptyGroup('Group 1')); };Add this helper right after the
interface Group(outside diffed ranges):function createEmptyGroup(title: string): Group { return { title, inputs: Array.from({ length: inputsName.value.length }, () => []), outputs: Array.from({ length: outputsName.value.length }, () => []), }; }Also applies to: 292-298, 354-366
267-280: Zero-fill new tests to the correct bitwidth per IO.Currently pushes "0" (width=1). Use the configured bitwidth per column.
const addTestToGroup = (index: number) => { const group = groups[index]; for (let i = 0; i < inputsBandWidth.value.length; i++) { if (group.inputs.length === i) group.inputs.push([]); - group.inputs[i].push("0"); + const inW = Math.max(1, Number(inputsBandWidth.value[i] ?? 1) | 0); + group.inputs[i].push("0".repeat(inW)); } for (let i = 0; i < outputsBandWidth.value.length; i++) { if (group.outputs.length === i) group.outputs.push([]); - group.outputs[i].push("0"); + const outW = Math.max(1, Number(outputsBandWidth.value[i] ?? 1) | 0); + group.outputs[i].push("0".repeat(outW)); } };
405-427: CSV import robustness: handle CRLF/blank lines, sanitize bitwidths, trim names, and strip trailing “:”.Fix parsing pitfalls that can cause NaN bitwidths, mis-labeled groups, or crashes on malformed lines.
- const csv = reader.result as string; - const lines = csv.split('\n'); + const csv = reader.result as string; + const lines = csv.split(/\r?\n/).filter(l => l.trim().length);- if (line) { - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } + if (line) { + const part = line.split(':')[1] ?? ''; + inputsBandWidth.value = part.split(',') + .map(v => Math.max(1, parseInt(v.trim(), 10) || 1)); + } line = lines.shift(); - if (line) { - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } + if (line) { + const part = line.split(':')[1] ?? ''; + outputsBandWidth.value = part.split(',') + .map(v => Math.max(1, parseInt(v.trim(), 10) || 1)); + } line = lines.shift(); - if (line) { - inputsName.value = line.split(': ')[1].split(','); - } + if (line) { + const part = line.split(':')[1] ?? ''; + inputsName.value = part.split(',').map(s => s.trim()).filter(Boolean); + } line = lines.shift(); - if (line) { - outputsName.value = line.split(': ')[1].split(','); - } + if (line) { + const part = line.split(':')[1] ?? ''; + outputsName.value = part.split(',').map(s => s.trim()).filter(Boolean); + }- if (line.startsWith('G-')) { + if (line.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { title: line.slice(2).replace(/:$/, '').trim(), inputs: [], outputs: [] }; } else { - const [name, values] = line.split(':'); - const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); - const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); - if (isInput) { - group.inputs.push(values.split(',')); - } else if (isOutput) { - group.outputs.push(values.split(',')); - } + const sepIdx = line.indexOf(':'); + if (sepIdx === -1) return; + const name = line.slice(0, sepIdx).trim(); + const valuesStr = line.slice(sepIdx + 1).trim(); + const base = name.slice(2); + if (name.startsWith('I-') && inputsName.value.includes(base)) { + group.inputs.push(valuesStr ? valuesStr.split(',') : []); + } else if (name.startsWith('O-') && outputsName.value.includes(base)) { + group.outputs.push(valuesStr ? valuesStr.split(',') : []); + } }Also applies to: 436-450
2-2: Fix v-model: computed returns a nested Ref, so dialog won’t close via overlay/ESC.Bind v-model directly to the store ref (or provide a get/set computed). As-is, v-model receives Ref<Ref> and won’t update.
- <v-dialog v-model="showCreator" :persistent="false" max-width="1100px"> + <v-dialog v-model="testBenchStore.showTestBenchCreator" :persistent="false" max-width="1100px">-const showCreator = computed(() => testBenchStore.showTestBenchCreator); +// Removed: bind v-model directly to the store ref aboveAlso applies to: 148-149
🧹 Nitpick comments (4)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (4)
232-256: Ensure bitWidth is numeric in payload.If users typed widths via text inputs earlier, these can be strings; coerce to numbers on send.
- bitWidth: inputsBandWidth.value[index], + bitWidth: Number(inputsBandWidth.value[index]),- bitWidth: outputsBandWidth.value[index], + bitWidth: Number(outputsBandWidth.value[index]),Would you like me to also add runtime clamping (min=1) before sending?
221-229: Use forEach index param instead of indexOf in the watcher.Minor perf/readability win.
- if (testType.value === 'comb') { - groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); - } - else { - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); - } + const prefix = testType.value === 'comb' ? 'Group' : 'Set'; + groups.forEach((group, idx) => { group.title = `${prefix} ${idx + 1}`; });
200-217: Replace confusing map-with-side-effects with forEach; pre-size results arrays.Improves clarity and avoids accidental misuse.
-watch(() => testBenchStore.testbenchData.testData.groups, () => { - results.splice(0, results.length); - testBenchStore.testbenchData.testData.groups.map(group => { - results.push([]); - group.outputs.map((output) => { - results[results.length - 1].push([]); - for(let i = 0; i < output.values.length; i++) { - if(output.results && output.values[i] === output.results[i]) { - results[results.length - 1][results[results.length - 1].length - 1].push(true); - } else { - results[results.length - 1][results[results.length - 1].length - 1].push(false); - } - } - }); - }); -}, +watch(() => testBenchStore.testbenchData.testData.groups, () => { + results.splice(0, results.length); + testBenchStore.testbenchData.testData.groups.forEach(group => { + const groupRes: boolean[][] = []; + group.outputs.forEach(output => { + const outRes: boolean[] = []; + const len = output.values.length; + for (let i = 0; i < len; i++) { + outRes.push(!!(output.results && output.values[i] === output.results[i])); + } + groupRes.push(outRes); + }); + results.push(groupRes); + }); +},
97-101: Use Vuetify 3 prepend-icon instead of embedding v-icon with “left”.Cleaner and consistent with v3 API.
- <v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)"> - <v-icon left>mdi-plus</v-icon> - Add Test - </v-btn> + <v-btn size="small" class="add-test-btn" variant="tonal" prepend-icon="mdi-plus" @click="addTestToGroup(groupIndex)"> + Add Test + </v-btn>- <v-btn class="action-btn new-group-btn" @click="addNewGroup"> - <v-icon left>mdi-plus</v-icon> - New Group - </v-btn> + <v-btn class="action-btn new-group-btn" prepend-icon="mdi-plus" @click="addNewGroup"> + New Group + </v-btn>Also applies to: 129-132
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(3 hunks)
🔇 Additional comments (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
144-147: Good: real store is used (mock removed).Importing and using the actual
useTestBenchStorerestores integration points like sendData, readOnly, and showResults.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (3)
300-313: “+ Inputs/Outputs” do nothing when columns are empty; also zero-fill should match bitwidthThe early return blocks adding the first column. Also, newly added cells should be zero-padded to their bitwidth.
Apply:
-const increInputs = () => { - groups.forEach((group) => { - if (group.inputs.length === 0) return; - - group.inputs.push([]); - - for (let i = 0; i < group.inputs[0].length; i++) { - group.inputs[group.inputs.length - 1].push("0"); - } - }); - - inputsBandWidth.value.push(1); - inputsName.value.push(`inp${inputsName.value.length + 1}`); -}; +const increInputs = () => { + const newW = Math.max(1, Number(inputsBandWidth.value.at(-1) ?? 1) || 1); + groups.forEach((group) => { + const testCount = group.inputs[0]?.length ?? 0; + const newCol = Array.from({ length: testCount }, () => "0".repeat(newW)); + group.inputs.push(newCol); + }); + inputsBandWidth.value.push(newW); + inputsName.value.push(`inp${inputsName.value.length + 1}`); +}; -const increOutputs = () => { - groups.forEach((group) => { - if (group.outputs.length === 0) return; - - group.outputs.push([]); - - for (let i = 0; i < group.outputs[0].length; i++) { - group.outputs[group.outputs.length - 1].push("0"); - } - }); - - outputsBandWidth.value.push(1); - outputsName.value.push(`out${outputsName.value.length + 1}`); -}; +const increOutputs = () => { + const newW = Math.max(1, Number(outputsBandWidth.value.at(-1) ?? 1) || 1); + groups.forEach((group) => { + const testCount = group.outputs[0]?.length ?? 0; + const newCol = Array.from({ length: testCount }, () => "0".repeat(newW)); + group.outputs.push(newCol); + }); + outputsBandWidth.value.push(newW); + outputsName.value.push(`out${outputsName.value.length + 1}`); +};Also applies to: 327-340
267-280: Add test rows using width-aware zero-fillEnsure each new cell matches the configured bitwidth.
const addTestToGroup = (index: number) => { const group = groups[index]; for (let i = 0; i < inputsBandWidth.value.length; i++) { - if (group.inputs.length === i) - group.inputs.push([]); - group.inputs[i].push("0"); + if (!group.inputs[i]) group.inputs[i] = []; + const inW = Math.max(1, Number(inputsBandWidth.value[i] ?? 1) || 1); + group.inputs[i].push("0".repeat(inW)); } for (let i = 0; i < outputsBandWidth.value.length; i++) { - if (group.outputs.length === i) - group.outputs.push([]); - group.outputs[i].push("0"); + if (!group.outputs[i]) group.outputs[i] = []; + const outW = Math.max(1, Number(outputsBandWidth.value[i] ?? 1) || 1); + group.outputs[i].push("0".repeat(outW)); } };
174-199: Sync from store: make watcher immediate and deepWithout
immediate, initial store data won’t hydrate the local state; withoutdeep, in-place mutations won’t be observed.-watch(() => testBenchStore.testbenchData.testData.groups, () => { +watch(() => testBenchStore.testbenchData.testData.groups, () => { // ...existing body... -}); +}, { deep: true, immediate: true });
♻️ Duplicate comments (4)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (4)
2-8: v-model binding fix looks goodBinding directly to the store ref resolves the nested Ref issue called out earlier. Overlay/ESC should now close the dialog reliably.
292-298: Initialize groups with proper column shapesNew/reset groups should start with one empty column per I/O name to keep the grid consistent.
const addNewGroup = () => { - groups.push({ - title: `Group ${groups.length + 1}`, - inputs: [], - outputs: [], - }); + const testCount = groups[0]?.inputs[0]?.length ?? groups[0]?.outputs[0]?.length ?? 0; + const newInputs = inputsName.value.map((_, idx) => + Array.from({ length: testCount }, () => "0".repeat(Math.max(1, Number(inputsBandWidth.value[idx] ?? 1) || 1))) + ); + const newOutputs = outputsName.value.map((_, idx) => + Array.from({ length: testCount }, () => "0".repeat(Math.max(1, Number(outputsBandWidth.value[idx] ?? 1) || 1))) + ); + groups.push({ + title: `Group ${groups.length + 1}`, + inputs: newInputs, + outputs: newOutputs, + }); }; const resetData = () => { testTitle.value = 'Untitled'; testType.value = 'comb'; inputsBandWidth.value = [1]; outputsBandWidth.value = [1]; inputsName.value = ["inp1"]; outputsName.value = ["out1"]; - groups.splice(0, groups.length, { - title: 'Group 1', - inputs: [], - outputs: [], - }); + groups.splice(0, groups.length, { + title: 'Group 1', + inputs: Array.from({ length: inputsName.value.length }, () => []), + outputs: Array.from({ length: outputsName.value.length }, () => []), + }); };Also applies to: 354-366
371-375: CSV import: sanitize widths, trim/CRLF-safe parsing, and strip trailing “:” from group titlesPrevents NaN/0 widths and parsing glitches on Windows or with empty lines.
-const importFromCSV = () => { +const importFromCSV = () => { // ... - reader.onload = () => { - const csv = reader.result as string; - const lines = csv.split('\n'); + reader.onload = () => { + const csv = reader.result as string; + const lines = csv.split(/\r?\n/).map(l => l.trim()).filter(l => l.length > 0); const firstLine = lines.shift(); if (firstLine) { [testType.value, testTitle.value] = firstLine.split(','); } - let line = lines.shift(); - if (line) { - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } - line = lines.shift(); - if (line) { - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } - line = lines.shift(); - if (line) { - inputsName.value = line.split(': ')[1].split(','); - } - line = lines.shift(); - if (line) { - outputsName.value = line.split(': ')[1].split(','); - } + let line = lines.shift(); + if (line?.startsWith('Inputs BandWidth')) { + inputsBandWidth.value = line.split(':')[1]?.split(',').map(v => Math.max(1, parseInt(v, 10) || 1)) ?? [1]; + } + line = lines.shift(); + if (line?.startsWith('Outputs BandWidth')) { + outputsBandWidth.value = line.split(':')[1]?.split(',').map(v => Math.max(1, parseInt(v, 10) || 1)) ?? [1]; + } + line = lines.shift(); + if (line?.startsWith('Inputs Name')) { + inputsName.value = line.split(':')[1]?.split(',').map(s => s.trim()).filter(Boolean) ?? ['inp1']; + } + line = lines.shift(); + if (line?.startsWith('Outputs Name')) { + outputsName.value = line.split(':')[1]?.split(',').map(s => s.trim()).filter(Boolean) ?? ['out1']; + } const newGroups: Group[] = []; let group: Group = { title: '', inputs: [], outputs: [], } lines.forEach(line => { if (line.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { title: line.slice(2).replace(/:\s*$/, ''), inputs: [], outputs: [] }; } else { - const [name, values] = line.split(':'); + const sep = line.indexOf(':'); + if (sep === -1) return; + const name = line.slice(0, sep).trim(); + const values = line.slice(sep + 1).trim(); const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); if (isInput) { - group.inputs.push(values.split(',')); + group.inputs.push(values ? values.split(',') : []); } else if (isOutput) { - group.outputs.push(values.split(',')); + group.outputs.push(values ? values.split(',') : []); } } }); if (group.title) { newGroups.push(group); } groups.splice(0, groups.length, ...newGroups); };Also applies to: 412-427, 429-457
103-116: Guard row render when no columns existWhen a group has zero input columns, iterating over
group.inputs[0]is brittle. Render against an empty array fallback to avoid edge-case warnings.- <div v-for="(_, testIndex) in group.inputs[0]" class="data-grid data-row" :key="testIndex" :class="{ 'with-results': testBenchStore.showResults }"> + <div v-for="(_, testIndex) in (group.inputs[0] || [])" class="data-grid data-row" :key="testIndex" :class="{ 'with-results': testBenchStore.showResults }">
🧹 Nitpick comments (9)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (9)
200-217: Derive results via computed (or make watcher immediate)This keeps results always in sync and simplifies state. If you keep the watcher, add
immediate: true.Computed option:
-const results: boolean[][][] = reactive([]); -// ...watch(...) to fill results... +const results = computed<boolean[][][]>(() => { + const out: boolean[][][] = []; + testBenchStore.testbenchData.testData.groups.forEach(group => { + const g: boolean[][] = []; + group.outputs.forEach(output => { + const r: boolean[] = []; + for (let i = 0; i < output.values.length; i++) { + r.push(!!(output.results && output.values[i] === output.results[i])); + } + g.push(r); + }); + out.push(g); + }); + return out; +});Or:
-); + , { deep: true, immediate: true });
83-89: Clamp bitwidths at input time to avoid NaN/unbounded maxlengthUsers can clear the number input, producing
null/NaN and removing maxlength. Clamp to [1,64] reactively.+watch([inputsBandWidth, outputsBandWidth], ([inBW, outBW]) => { + const clamp = (v: unknown) => Math.min(64, Math.max(1, Number(v) || 1)); + inBW.forEach((v, i) => { const c = clamp(v); if (c !== v) inputsBandWidth.value[i] = c; }); + outBW.forEach((v, i) => { const c = clamp(v); if (c !== v) outputsBandWidth.value[i] = c; }); +}, { deep: true });Also applies to: 109-115
250-256: Compute n safely across inputs/outputs
nbased only on the first input can be wrong if arrays differ. Use the max across all I/Os.- n: inputsData[0] ? inputsData[0].values.length : 0, + n: (() => { + const counts = [...inputsData, ...outputsData].map(io => io.values.length); + return counts.length ? Math.max(...counts) : 0; + })(),
48-53: Add accessible labels to icon-only buttonsImproves screen-reader usability without UI changes.
-<v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" @click="increInputs"><v-icon>mdi-plus</v-icon></v-btn> +<v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" aria-label="Add input column" @click="increInputs"><v-icon>mdi-plus</v-icon></v-btn> -<v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" @click="increOutputs"><v-icon>mdi-plus</v-icon></v-btn> +<v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" aria-label="Add output column" @click="increOutputs"><v-icon>mdi-plus</v-icon></v-btn> -<v-btn icon size="x-small" variant="text" class="delete-io-btn" @click="deleteInput(i)"><v-icon size="small">mdi-minus-circle-outline</v-icon></v-btn> +<v-btn icon size="x-small" variant="text" class="delete-io-btn" aria-label="Remove input column" @click="deleteInput(i)"><v-icon size="small">mdi-minus-circle-outline</v-icon></v-btn> -<v-btn icon size="x-small" variant="text" class="delete-io-btn" @click="deleteOutput(i)"><v-icon size="small">mdi-minus-circle-outline</v-icon></v-btn> +<v-btn icon size="x-small" variant="text" class="delete-io-btn" aria-label="Remove output column" @click="deleteOutput(i)"><v-icon size="small">mdi-minus-circle-outline</v-icon></v-btn> -<v-btn icon size="x-small" variant="text" class="delete-io-btn" @click="deleteTestFromGroup(groupIndex, testIndex)"><v-icon size="small">mdi-close</v-icon></v-btn> +<v-btn icon size="x-small" variant="text" class="delete-io-btn" aria-label="Remove test row" @click="deleteTestFromGroup(groupIndex, testIndex)"><v-icon size="small">mdi-close</v-icon></v-btn>Also applies to: 67-68, 73-74, 104-106
219-230: Minor: avoid indexOf in a loopUse the loop index for O(n)→O(n) with fewer lookups.
-if (testType.value === 'comb') { - groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); -} -else { - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); -} +groups.forEach((group, idx) => { + group.title = testType.value === 'comb' ? `Group ${idx + 1}` : `Set ${idx + 1}`; +});
368-392: Optional: quote CSV fields with commas/newlinesSafer CSV for titles containing commas.
-const exportAsCSV = () => { - let csv = `${testType.value},${testTitle.value}\n`; +const exportAsCSV = () => { + const esc = (s: string) => /[",\n]/.test(s) ? `"${String(s).replace(/"/g, '""')}"` : s; + let csv = `${esc(testType.value)},${esc(testTitle.value)}\n`;
232-266: Consider closing the dialog after attach (if that’s the UX spec)Optionally set
showTestBenchCreator = falseaftersendData.
48-56: UX polish: disable structure-edit controls in readOnlyHide/disable add/remove buttons when
readOnlyis true to prevent confusing no-ops.-<v-btn icon ... @click="increInputs"> +<v-btn icon :disabled="testBenchStore.readOnly" ... @click="increInputs"> ... -<v-btn icon ... @click="increOutputs"> +<v-btn icon :disabled="testBenchStore.readOnly" ... @click="increOutputs"> ... -<v-btn icon ... @click="deleteInput(i)"> +<v-btn icon :disabled="testBenchStore.readOnly" ... @click="deleteInput(i)"> ... -<v-btn icon ... @click="deleteOutput(i)"> +<v-btn icon :disabled="testBenchStore.readOnly" ... @click="deleteOutput(i)"> ... -<v-btn class="action-btn new-group-btn" @click="addNewGroup"> +<v-btn class="action-btn new-group-btn" :disabled="testBenchStore.readOnly" @click="addNewGroup">Also applies to: 60-76, 78-91, 129-137
97-101: Use the prepend-icon prop on v-btn instead of nested<v-icon>
Vuetify 3 supports the v-btnprepend-iconprop for button icons. In TestBenchCreator.vue (lines 97–101 and 129–132), refactor as shown:@@ TestBenchCreator.vue:97-101 -<v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)"> - <v-icon left>mdi-plus</v-icon> - Add Test -</v-btn> +<v-btn size="small" class="add-test-btn" variant="tonal" prepend-icon="mdi-plus" @click="addTestToGroup(groupIndex)"> + Add Test +</v-btn> @@ TestBenchCreator.vue:129-132 -<v-btn class="action-btn new-group-btn" @click="addNewGroup"> - <v-icon left>mdi-plus</v-icon> - New Group -</v-btn> +<v-btn class="action-btn new-group-btn" prepend-icon="mdi-plus" @click="addNewGroup"> + New Group +</v-btn>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(4 hunks)
|
@gr455 @niladrix719 PTAL |
|
@nitin10s PTAL |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (3)
161-185: Make store sync and results computation run on mount.These watches won’t run until the store mutates. Use immediate to hydrate UI on open.
-watch(() => testBenchStore.testbenchData.testData.groups, () => { +watch(() => testBenchStore.testbenchData.testData.groups, () => { // ... -}); +}, { immediate: true });-watch(() => testBenchStore.testbenchData.testData.groups, () => { +watch(() => testBenchStore.testbenchData.testData.groups, () => { // ... -}, - { deep: true } -); +}, { deep: true, immediate: true });Also applies to: 187-204
219-243: Compute n robustly (fallback to outputs) and verify alignment.If there are no inputs (but outputs exist), n becomes 0. Use max across first input/output lengths.
- return { - label: group.title, - inputs: inputsData, - outputs: outputsData, - n: inputsData[0] ? inputsData[0].values.length : 0, - }; + const nIn = inputsData[0]?.values.length ?? 0; + const nOut = outputsData[0]?.values.length ?? 0; + return { + label: group.title, + inputs: inputsData, + outputs: outputsData, + n: Math.max(nIn, nOut), + };
399-414: CSV import: fix group title parsing, sanitize headers, and align by IO name.
- Titles retain the trailing colon (G-Group 1:). Trim it.
- Bitwidth parsing can yield NaN/0; clamp to >=1.
- Map I-/O- lines to the correct index based on header names (don’t rely on file order).
- const firstLine = lines.shift(); + const firstLine = lines.shift(); if (firstLine) { - [testType.value, testTitle.value] = firstLine.split(','); + const [t, ...rest] = firstLine.split(','); + testType.value = t?.trim() || 'comb'; + testTitle.value = rest.join(',').trim() || 'Untitled'; } @@ - if (line) { - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } + if (line) { + const rhs = line.split(/:\s*/)[1] ?? ''; + inputsBandWidth.value = rhs.split(',').map(v => Math.max(1, parseInt(v, 10) || 1)); + } @@ - if (line) { - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } + if (line) { + const rhs = line.split(/:\s*/)[1] ?? ''; + outputsBandWidth.value = rhs.split(',').map(v => Math.max(1, parseInt(v, 10) || 1)); + } @@ - if (line) { - inputsName.value = line.split(': ')[1].split(','); - } + if (line) { + const rhs = line.split(/:\s*/)[1] ?? ''; + inputsName.value = rhs.split(',').map(s => s.trim()).filter(Boolean); + } @@ - if (line) { - outputsName.value = line.split(': ')[1].split(','); - } + if (line) { + const rhs = line.split(/:\s*/)[1] ?? ''; + outputsName.value = rhs.split(',').map(s => s.trim()).filter(Boolean); + } @@ - if (line.startsWith('G-')) { + const trimmed = line.trim(); + if (!trimmed) return; + if (trimmed.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { + title: trimmed.slice(2).replace(/:\s*$/, ''), + inputs: Array.from({ length: inputsName.value.length }, () => []), + outputs: Array.from({ length: outputsName.value.length }, () => []), + }; } else { - const [name, values] = line.split(':'); + const [name, valuesRaw] = trimmed.split(':'); + if (!name || valuesRaw === undefined) return; + const values = valuesRaw.split(',').map(s => s.trim()); const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); - if (isInput) { - group.inputs.push(values.split(',')); - } else if (isOutput) { - group.outputs.push(values.split(',')); - } + if (isInput) { + const idx = inputsName.value.indexOf(name.slice(2)); + group.inputs[idx] = values; + } else if (isOutput) { + const idx = outputsName.value.indexOf(name.slice(2)); + group.outputs[idx] = values; + } } }); if (group.title) { newGroups.push(group); }Also applies to: 423-441
🧹 Nitpick comments (3)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (3)
254-267: Zero-fill to bitwidth when adding a test.Cells default to "0" (length 1). Fill with correct width.
- for (let i = 0; i < inputsBandWidth.value.length; i++) { + for (let i = 0; i < inputsBandWidth.value.length; i++) { if (group.inputs.length === i) group.inputs.push([]); - group.inputs[i].push("0"); + const w = Math.max(1, Number(inputsBandWidth.value[i] ?? 1) | 0); + group.inputs[i].push("0".repeat(w)); } @@ - for (let i = 0; i < outputsBandWidth.value.length; i++) { + for (let i = 0; i < outputsBandWidth.value.length; i++) { if (group.outputs.length === i) group.outputs.push([]); - group.outputs[i].push("0"); + const w = Math.max(1, Number(outputsBandWidth.value[i] ?? 1) | 0); + group.outputs[i].push("0".repeat(w)); }
287-300: Allow adding IO columns even when there are no tests; size new cols to current test count.Early-return prevents column additions on empty groups; also zero-fill to bitwidth where possible.
- groups.forEach((group) => { - if (group.inputs.length === 0) return; - - group.inputs.push([]); - - for (let i = 0; i < group.inputs[0].length; i++) { - group.inputs[group.inputs.length - 1].push("0"); - } - }); + groups.forEach((group) => { + const testCount = group.inputs[0]?.length ?? 0; + const newColIdx = group.inputs.length; + const w = Math.max(1, Number(inputsBandWidth.value[newColIdx] ?? 1) | 0); + const fill = "0".repeat(w); + group.inputs.push(Array.from({ length: testCount }, () => fill)); + });- groups.forEach((group) => { - if (group.outputs.length === 0) return; - - group.outputs.push([]); - - for (let i = 0; i < group.outputs[0].length; i++) { - group.outputs[group.outputs.length - 1].push("0"); - } - }); + groups.forEach((group) => { + const testCount = group.outputs[0]?.length ?? 0; + const newColIdx = group.outputs.length; + const w = Math.max(1, Number(outputsBandWidth.value[newColIdx] ?? 1) | 0); + const fill = "0".repeat(w); + group.outputs.push(Array.from({ length: testCount }, () => fill)); + });Also applies to: 314-327
98-104: Optionally restrict data input format to binary and surface invalid entry.If values must be binary, add pattern/inputmode and visual invalid state.
-<input class="io-input data-input" type="text" v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="inputsBandWidth[i]" /> +<input class="io-input data-input" type="text" inputmode="numeric" pattern="[01]*" v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="inputsBandWidth[i]" />-<input class="io-input data-input" type="text" v-model="group.outputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="outputsBandWidth[i]" /> +<input class="io-input data-input" type="text" inputmode="numeric" pattern="[01]*" v-model="group.outputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="outputsBandWidth[i]" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (4)
2-7: Dialog binding fix looks correct.v-model now targets the actual store ref and the close button updates it. Overlay/ESC should work.
206-217: Switching test type auto-renames groups, overriding user titles. Intended?Confirm UX: should manual titles persist when toggling seq/comb?
69-79: Bitwidth inputs: good move to numeric with bounds..number + min/max prevents RangeError on repeat and keeps UI sane.
453-717: Styling overhaul is clean and cohesive.Grid layout, clear tokens, hover affordances, and footer actions improve UX significantly.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (4)
254-266: Zero-fill to the declared bitwidth when adding tests/columns.Currently pushes a single "0", producing wrong-width vectors.
Apply:
const addTestToGroup = (index: number) => { const group = groups[index]; for (let i = 0; i < inputsBandWidth.value.length; i++) { - if (group.inputs.length === i) - group.inputs.push([]); - group.inputs[i].push("0"); + if (group.inputs.length === i) group.inputs.push([]); + group.inputs[i].push("0".repeat(inWidth(i))); } for (let i = 0; i < outputsBandWidth.value.length; i++) { - if (group.outputs.length === i) - group.outputs.push([]); - group.outputs[i].push("0"); + if (group.outputs.length === i) group.outputs.push([]); + group.outputs[i].push("0".repeat(outWidth(i))); } };const increInputs = () => { groups.forEach((group) => { if (group.inputs.length === 0) return; group.inputs.push([]); - for (let i = 0; i < group.inputs[0].length; i++) { - group.inputs[group.inputs.length - 1].push("0"); - } + const newIdx = group.inputs.length - 1; + for (let i = 0; i < (group.inputs[0]?.length ?? 0); i++) { + group.inputs[newIdx].push("0".repeat(inWidth(newIdx))); + } }); inputsBandWidth.value.push(1); inputsName.value.push(`inp${inputsName.value.length + 1}`); };const increOutputs = () => { groups.forEach((group) => { if (group.outputs.length === 0) return; group.outputs.push([]); - for (let i = 0; i < group.outputs[0].length; i++) { - group.outputs[group.outputs.length - 1].push("0"); - } + const newIdx = group.outputs.length - 1; + for (let i = 0; i < (group.outputs[0]?.length ?? 0); i++) { + group.outputs[newIdx].push("0".repeat(outWidth(newIdx))); + } }); outputsBandWidth.value.push(1); outputsName.value.push(`out${outputsName.value.length + 1}`); };Also applies to: 287-300, 314-327
392-415: CSV header parsing: trim, clamp bitwidths to [1,64], and sanitize names.Prevents NaN/0 widths and stray whitespace from corrupting shapes.
Apply:
- const lines = csv.split('\n'); + const lines = csv.split('\n').map(l => l.replace(/\r$/, '')); const firstLine = lines.shift(); if (firstLine) { - [testType.value, testTitle.value] = firstLine.split(','); + const [t, title] = firstLine.split(','); + testType.value = (t || '').trim(); + testTitle.value = (title || '').trim(); } let line = lines.shift(); if (line) { - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); + inputsBandWidth.value = (line.split(': ')[1] || '') + .split(',') + .map(v => Math.max(1, Math.min(64, parseInt(v.trim(), 10) || 1))); } line = lines.shift(); if (line) { - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); + outputsBandWidth.value = (line.split(': ')[1] || '') + .split(',') + .map(v => Math.max(1, Math.min(64, parseInt(v.trim(), 10) || 1))); } line = lines.shift(); if (line) { - inputsName.value = line.split(': ')[1].split(','); + inputsName.value = (line.split(': ')[1] || '') + .split(',') + .map(v => v.trim()) + .filter(Boolean); } line = lines.shift(); if (line) { - outputsName.value = line.split(': ')[1].split(','); + outputsName.value = (line.split(': ')[1] || '') + .split(',') + .map(v => v.trim()) + .filter(Boolean); }
423-444: CSV group parsing: strip trailing colon in G-lines, handle empty value lists, preserve I/O order, and backfill missing columns.Prevents off-by-one/shape drift and keeps columns aligned with header labels.
Apply:
- lines.forEach(line => { + lines.forEach(line => { if (line.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { title: line.slice(2).replace(/:$/, '').trim(), inputs: [], outputs: [] }; } else { - const [name, values] = line.split(':'); - const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); - const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); + const [nameRaw, valuesRaw = ''] = line.split(':'); + const name = (nameRaw || '').trim(); + const values = (valuesRaw || '').trim(); + const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); + const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); if (isInput) { - group.inputs.push(values.split(',')); + const idx = inputsName.value.indexOf(name.slice(2)); + const arr = values ? values.split(',').map(v => v.trim()).filter(Boolean) : []; + group.inputs[idx] = arr; } else if (isOutput) { - group.outputs.push(values.split(',')); + const idx = outputsName.value.indexOf(name.slice(2)); + const arr = values ? values.split(',').map(v => v.trim()).filter(Boolean) : []; + group.outputs[idx] = arr; } } }); if (group.title) { newGroups.push(group); } - groups.splice(0, groups.length, ...newGroups); + newGroups.forEach(g => { + g.inputs = Array.from({ length: inputsName.value.length }, (_, i) => g.inputs[i] || []); + g.outputs = Array.from({ length: outputsName.value.length }, (_, i) => g.outputs[i] || []); + }); + groups.splice(0, groups.length, ...newGroups);
161-185: Hydrate from store on mount; also build results immediately.Without
immediate: true, pre-existing store data won’t populate the dialog on first open.Apply:
-watch(() => testBenchStore.testbenchData.testData.groups, () => { +watch(() => testBenchStore.testbenchData.testData.groups, () => { // ...unchanged... -}); +}, { immediate: true });-watch(() => testBenchStore.testbenchData.testData.groups, () => { +watch(() => testBenchStore.testbenchData.testData.groups, () => { // ...unchanged... -}, - { deep: true } -); +}, { deep: true, immediate: true });Also applies to: 187-205
🧹 Nitpick comments (5)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (5)
36-46: Respect readOnly/showResults for mutating controls and inputs.Disable add/delete/reset/import/attach and non-data inputs while read-only or showing results to prevent state drift.
Example:
-<v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" @click="increInputs"><v-icon>mdi-plus</v-icon></v-btn> +<v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" @click="increInputs" :disabled="testBenchStore.readOnly || testBenchStore.showResults" aria-label="Add input column"><v-icon>mdi-plus</v-icon></v-btn>-<input class="io-input" type="text" v-model="inputsName[i]" /> +<input class="io-input" type="text" v-model="inputsName[i]" :disabled="testBenchStore.readOnly || testBenchStore.showResults" />-<input class="io-input bitwidth-input" type="number" v-model.number="inputsBandWidth[i]" min="1" max="64" /> +<input class="io-input bitwidth-input" type="number" v-model.number="inputsBandWidth[i]" min="1" max="64" :disabled="testBenchStore.readOnly || testBenchStore.showResults" />-<v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)"> +<v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)" :disabled="testBenchStore.readOnly || testBenchStore.showResults">-<v-btn class="action-btn new-group-btn" @click="addNewGroup"> +<v-btn class="action-btn new-group-btn" @click="addNewGroup" :disabled="testBenchStore.readOnly || testBenchStore.showResults">Also applies to: 51-67, 69-81, 83-91, 116-124
86-90: Vuetify 3 cleanup: use prepend-icon instead of deprecatedv-icon leftinside buttons.Removes dead props and aligns with v3 API.
Apply:
-<v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)"> - <v-icon left>mdi-plus</v-icon> - Add Test -</v-btn> +<v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)" prepend-icon="mdi-plus"> + Add Test +</v-btn>-<v-btn class="action-btn new-group-btn" @click="addNewGroup"> - <v-icon left>mdi-plus</v-icon> - New Group -</v-btn> +<v-btn class="action-btn new-group-btn" @click="addNewGroup" prepend-icon="mdi-plus"> + New Group +</v-btn>Also applies to: 117-120
341-353: Reset should clear computed results for a clean slate.Avoids stale result badges after reset.
Apply:
const resetData = () => { // ...unchanged... - groups.splice(0, groups.length, { + groups.splice(0, groups.length, { title: 'Group 1', inputs: Array.from({ length: inputsName.value.length }, () => []), outputs: Array.from({ length: outputsName.value.length }, () => []), }); + results.splice(0, results.length); };
206-217: Minor: avoid O(n²) name updates and duplicate lookups.Use the forEach index to label groups.
Apply:
-watch(testType, () => { - if (testType.value === 'comb') { - groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); - } - else { - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); - } -}); +watch(testType, () => { + groups.forEach((_, idx) => { + groups[idx].title = testType.value === 'comb' ? `Group ${idx + 1}` : `Set ${idx + 1}`; + }); +});
355-379: Optional: CSV export hardening (Excel injection).Escape leading =,+,-,@ to avoid formula execution when opened in spreadsheets.
Add helper and use it for the title:
const escapeCSVField = (s: string) => s.replace(/^\s*([=+\-@])/,'\'$1');- let csv = `${testType.value},${testTitle.value}\n`; + let csv = `${testType.value},${escapeCSVField(testTitle.value)}\n`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - circuitverse
- GitHub Check: Header rules - circuitverse
- GitHub Check: Pages changed - circuitverse
🔇 Additional comments (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
2-7: Good fix: bind dialog directly to the store ref.This resolves the nested Ref issue and enables overlay/ESC close properly.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (5)
266-279: Zero-fill new tests to bitwidth, not a single charPushing "0" leads to width mismatches and failing comparisons.
Apply:
for (let i = 0; i < inputsBandWidth.value.length; i++) { if (group.inputs.length === i) group.inputs.push([]); - group.inputs[i].push("0"); + group.inputs[i].push("0".repeat(inWidth(i))); } ... for (let i = 0; i < outputsBandWidth.value.length; i++) { if (group.outputs.length === i) group.outputs.push([]); - group.outputs[i].push("0"); + group.outputs[i].push("0".repeat(outWidth(i))); }
299-312: When adding an input column, fill each existing test using the new column’s widthCurrently fills with single "0". Use the effective width.
Apply:
groups.forEach((group) => { if (group.inputs.length === 0) return; - group.inputs.push([]); - - for (let i = 0; i < group.inputs[0].length; i++) { - group.inputs[group.inputs.length - 1].push("0"); - } + const testCount = group.inputs[0].length; + const newW = clampW(inputsBandWidth.value.at(-1) ?? 1); + const newCol = Array.from({ length: testCount }, () => "0".repeat(newW)); + group.inputs.push(newCol); }); inputsBandWidth.value.push(1); inputsName.value.push(`inp${inputsName.value.length + 1}`);
326-335: Same for outputs: width-aware zero-fill for new columnMirror the input fix.
Apply:
groups.forEach((group) => { if (group.outputs.length === 0) return; - group.outputs.push([]); - - for (let i = 0; i < group.outputs[0].length; i++) { - group.outputs[group.outputs.length - 1].push("0"); - } + const testCount = group.outputs[0].length; + const newW = clampW(outputsBandWidth.value.at(-1) ?? 1); + const newCol = Array.from({ length: testCount }, () => "0".repeat(newW)); + group.outputs.push(newCol); }); outputsBandWidth.value.push(1); outputsName.value.push(`out${outputsName.value.length + 1}`);
367-381: CSV export: remove trailing colon from group headerImport currently keeps the colon in the title (line 439). Drop it during export.
Apply:
- csv += groups.map(group => { - let groupStr = `G-${group.title}:\n`; + csv += groups.map(group => { + let groupStr = `G-${group.title}\n`;
435-449: CSV import: trim lines and strip optional trailing colon in group headersPrevents titles like “Group 1:” and ignores blank lines.
Apply:
- lines.forEach(line => { - if (line.startsWith('G-')) { + lines.forEach(raw => { + const line = raw.trim(); + if (!line) return; + if (line.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { title: line.slice(2).replace(/:\s*$/, ''), inputs: [], outputs: [] }; } else { - const [name, values] = line.split(':'); + const [name, values] = line.split(':'); const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); if (isInput) { - group.inputs.push(values.split(',')); + group.inputs.push((values ?? '').split(',').filter(v => v.length)); } else if (isOutput) { - group.outputs.push(values.split(',')); + group.outputs.push((values ?? '').split(',').filter(v => v.length)); } } });
♻️ Duplicate comments (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
411-426: CSV import: sanitize bitwidths to >= 1 and guard parsingAvoid NaN/0 widths which later break repeat(). This was previously raised.
Apply:
- if (line) { - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } + if (line) { + const rhs = line.split(': ')[1] ?? ''; + inputsBandWidth.value = rhs.split(',').map(v => clampW(parseInt(v, 10))); + } ... - if (line) { - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } + if (line) { + const rhs = line.split(': ')[1] ?? ''; + outputsBandWidth.value = rhs.split(',').map(v => clampW(parseInt(v, 10))); + } ... - if (line) { - inputsName.value = line.split(': ')[1].split(','); - } + if (line) { + const rhs = line.split(': ')[1] ?? ''; + inputsName.value = rhs.split(',').map(s => s.trim()).filter(Boolean); + } ... - if (line) { - outputsName.value = line.split(': ')[1].split(','); - } + if (line) { + const rhs = line.split(': ')[1] ?? ''; + outputsName.value = rhs.split(',').map(s => s.trim()).filter(Boolean); + }
🧹 Nitpick comments (4)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (4)
36-45: Disable add-column actions in read-only modePrevent accidental edits when the panel is read-only.
Apply:
-<div class="grid-cell header-cell inputs-col"> +<div class="grid-cell header-cell inputs-col"> <span>Inputs</span> - <v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" @click="increInputs"><v-icon>mdi-plus</v-icon></v-btn> + <v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" :disabled="testBenchStore.readOnly" @click="increInputs"><v-icon>mdi-plus</v-icon></v-btn> </div> ... -<div class="grid-cell header-cell outputs-col"> +<div class="grid-cell header-cell outputs-col"> <span>Outputs</span> - <v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" @click="increOutputs"><v-icon>mdi-plus</v-icon></v-btn> + <v-btn icon size="x-small" variant="flat" class="ml-2 action-icon" :disabled="testBenchStore.readOnly" @click="increOutputs"><v-icon>mdi-plus</v-icon></v-btn> </div>
218-229: Avoid O(n²) title updatesUse the index param of forEach instead of indexOf inside the loop.
Apply:
-if (testType.value === 'comb') { - groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); -} else { - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); -} +if (testType.value === 'comb') { + groups.forEach((group, idx) => { group.title = `Group ${idx + 1}`; }); +} else { + groups.forEach((group, idx) => { group.title = `Set ${idx + 1}`; }); +}
110-116: Optional: enforce binary-only input and auto-pad to widthIf values must be binary strings, add inputmode/pattern and normalize on blur.
Example template tweak:
-<input class="io-input data-input" type="text" v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="inWidth(i)" /> +<input class="io-input data-input" type="text" inputmode="numeric" pattern="[01]*" + v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="inWidth(i)" />And a normalizer (optional) in <script setup>:
const norm = (s: string, w: number) => s.replace(/[^01]/g, '').slice(0, w).padStart(w, '0');Also applies to: 69-91
69-91: Optional: clamp number inputs on manual editsEven with min/max, v-model.number can set 0/NaN. Deep-watch and clamp.
Add in <script setup>:
watch(inputsBandWidth, (arr) => { arr.forEach((v, i) => inputsBandWidth.value[i] = clampW(v)); }, { deep: true }); watch(outputsBandWidth, (arr) => { arr.forEach((v, i) => outputsBandWidth.value[i] = clampW(v)); }, { deep: true });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(5 hunks)
🔇 Additional comments (3)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (3)
2-6: LGTM: dialog binding fixed to real storeBinding v-model directly to testBenchStore.showTestBenchCreator and the close button setter looks correct.
165-171: LGTM: groups initialized with per-IO column arraysPrevents undefined access for group.inputs[0]/outputs[0] in the template.
353-365: LGTM: resetData reinitializes shape safelyResets refs and recreates per-IO columns—solid.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
239-246: Send numeric bitWidth in payloadCoerce to numbers to match store schema and avoid leaking strings when edited via inputs.
const inputsData = group.inputs.map((input, index) => { return { label: inputsName.value[index], - bitWidth: inputsBandWidth.value[index], + bitWidth: Number(inputsBandWidth.value[index]), values: input }; }); const outputsData = group.outputs.map((output, index) => { return { label: outputsName.value[index], - bitWidth: outputsBandWidth.value[index], + bitWidth: Number(outputsBandWidth.value[index]), values: output }; });Also applies to: 247-254
src/components/Panels/TestBenchPanel/TestBenchCreator.vue (2)
239-246: Coerce bitWidth to numbers in payloadAvoid string bitWidth leaking into store.
- bitWidth: inputsBandWidth.value[index], + bitWidth: Number(inputsBandWidth.value[index]), @@ - bitWidth: outputsBandWidth.value[index], + bitWidth: Number(outputsBandWidth.value[index]),Also applies to: 247-254
1-743: Remove duplicate TestBenchCreator component copy
The fileTestBenchCreator.vueexists in bothv1/src/components/Panels/TestBenchPanel/andsrc/components/Panels/TestBenchPanel/; remove one copy (or consolidate into a single shared location) to prevent future drift.
🧹 Nitpick comments (14)
v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (8)
158-163: Type and harden clamp()Make return type explicit, accept unknown, and normalize to finite integers.
-const clamp = (val) => { - if (!val) return 1 - const n = Number(val) - if (isNaN(n)) return 1 - return Math.min(Math.max(n, 1), 64) -} +const clamp = (val: unknown): number => { + const n = Number(val); + if (!Number.isFinite(n)) return 1; + return Math.min(Math.max(Math.trunc(n), 1), 64); +};
205-223: Use forEach instead of map for side effectsmap’s return value is unused; forEach signals intent and avoids lint noise.
- testBenchStore.testbenchData.testData.groups.map(group => { + testBenchStore.testbenchData.testData.groups.forEach(group => { results.push([]); - group.outputs.map((output) => { + group.outputs.forEach((output) => { results[results.length - 1].push([]); for(let i = 0; i < output.values.length; i++) { if(output.results && output.values[i] === output.results[i]) { results[results.length - 1][results[results.length - 1].length - 1].push(true); } else { results[results.length - 1][results[results.length - 1].length - 1].push(false); } } - }); - }); + }); + });
224-235: Avoid O(n²) indexOf in renaming loopUse the forEach index parameter.
- if (testType.value === 'comb') { - groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); - } - else { - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); - } + if (testType.value === 'comb') { + groups.forEach((group, idx) => { group.title = `Group ${idx + 1}`; }); + } else { + groups.forEach((group, idx) => { group.title = `Set ${idx + 1}`; }); + }
98-101: Vuetify 3: prefer prepend-icon on v-btnleft prop on v-icon is Vuetify 2-era. Use prepend-icon for cleaner markup.
-<v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)"> - <v-icon left>mdi-plus</v-icon> - Add Test -</v-btn> +<v-btn size="small" class="add-test-btn" variant="tonal" prepend-icon="mdi-plus" @click="addTestToGroup(groupIndex)"> + Add Test +</v-btn>
417-433: CSV import robustness: trim lines and strip trailing colon from group titlesHandle CRLF/extra whitespace, skip malformed rows, and remove trailing ":" from G- titles to avoid persisting it in the UI.
- let line = lines.shift(); + let line = lines.shift(); if (line) { - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); + const rhs = line.split(': ')[1]?.trim(); + if (rhs) inputsBandWidth.value = rhs.split(',').map(Number); } line = lines.shift(); if (line) { - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); + const rhs = line.split(': ')[1]?.trim(); + if (rhs) outputsBandWidth.value = rhs.split(',').map(Number); } line = lines.shift(); if (line) { - inputsName.value = line.split(': ')[1].split(','); + const rhs = line.split(': ')[1]?.trim(); + if (rhs) inputsName.value = rhs.split(','); } line = lines.shift(); if (line) { - outputsName.value = line.split(': ')[1].split(','); + const rhs = line.split(': ')[1]?.trim(); + if (rhs) outputsName.value = rhs.split(','); } @@ - lines.forEach(line => { - if (line.startsWith('G-')) { + lines.forEach(rawLine => { + const line = rawLine.trim(); + if (!line) return; + if (line.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { title: line.slice(2).replace(/:$/, ''), inputs: [], outputs: [] }; } else { - const [name, values] = line.split(':'); + const [name, values] = line.split(':'); + if (!name || values === undefined) return; const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); if (isInput) { group.inputs.push(values.split(',')); } else if (isOutput) { group.outputs.push(values.split(',')); } } });Also applies to: 441-456
149-149: Type results with reactive() explicitlyHelps Volar/TS tooling and avoids implicit any in downstream template access.
-const results: boolean[][][] = reactive([]); +const results = reactive<boolean[][][]>([] as boolean[][][]);
373-387: CSV key naming: BandWidth vs BitWidthExport uses “BandWidth” (typo); importer assumes positional lines, so it works but isn’t self-describing. Consider tolerating both “BitWidth” and “BandWidth” on import without changing export for backward compat.
I can follow up with a small parser that detects keys instead of relying on order.
Also applies to: 399-468
1-743: Consolidate duplicate components
Both v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue and src/components/Panels/TestBenchPanel/TestBenchCreator.vue are identical; remove one copy or centralize into a shared path to eliminate duplication (or add a CI sync check) to prevent future drift.src/components/Panels/TestBenchPanel/TestBenchCreator.vue (6)
158-163: Type clamp() and normalize to integers-const clamp = (val) => { - if (!val) return 1 - const n = Number(val) - if (isNaN(n)) return 1 - return Math.min(Math.max(n, 1), 64) -} +const clamp = (val: unknown): number => { + const n = Number(val); + if (!Number.isFinite(n)) return 1; + return Math.min(Math.max(Math.trunc(n), 1), 64); +};
205-223: Use forEach for side effects in results watcher- testBenchStore.testbenchData.testData.groups.map(group => { + testBenchStore.testbenchData.testData.groups.forEach(group => { results.push([]); - group.outputs.map((output) => { + group.outputs.forEach((output) => {
224-235: Rename groups using forEach index- groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); + groups.forEach((group, idx) => { group.title = `Group ${idx + 1}`; }); @@ - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); + groups.forEach((group, idx) => { group.title = `Set ${idx + 1}`; });
98-101: Prefer prepend-icon on v-btn (Vuetify 3)Cleaner than embedding v-icon with the legacy left prop.
-<v-btn size="small" class="add-test-btn" variant="tonal" @click="addTestToGroup(groupIndex)"> - <v-icon left>mdi-plus</v-icon> - Add Test -</v-btn> +<v-btn size="small" class="add-test-btn" variant="tonal" prepend-icon="mdi-plus" @click="addTestToGroup(groupIndex)"> + Add Test +</v-btn>
417-433: CSV import: trim lines, tolerate trailing colonPrevents carrying ":" into titles and skips malformed lines.
- let line = lines.shift(); + let line = lines.shift(); if (line) { - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); + const rhs = line.split(': ')[1]?.trim(); + if (rhs) inputsBandWidth.value = rhs.split(',').map(Number); } line = lines.shift(); if (line) { - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); + const rhs = line.split(': ')[1]?.trim(); + if (rhs) outputsBandWidth.value = rhs.split(',').map(Number); } line = lines.shift(); if (line) { - inputsName.value = line.split(': ')[1].split(','); + const rhs = line.split(': ')[1]?.trim(); + if (rhs) inputsName.value = rhs.split(','); } line = lines.shift(); if (line) { - outputsName.value = line.split(': ')[1].split(','); + const rhs = line.split(': ')[1]?.trim(); + if (rhs) outputsName.value = rhs.split(','); } @@ - lines.forEach(line => { - if (line.startsWith('G-')) { + lines.forEach(rawLine => { + const line = rawLine.trim(); + if (!line) return; + if (line.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { title: line.slice(2).replace(/:$/, ''), inputs: [], outputs: [] }; } else { - const [name, values] = line.split(':'); + const [name, values] = line.split(':'); + if (!name || values === undefined) return; const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2));Also applies to: 441-456
149-149: Explicit reactive typing for results-const results: boolean[][][] = reactive([]); +const results = reactive<boolean[][][]>([] as boolean[][][]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
110-115: Confirm intent of maxlength vs bit-widthmaxlength limits character count, not numeric value range. If values are binary strings, consider pattern="[01]*" and enforcing exact length; if hex/dec allowed, validate differently.
Do you want strict binary entry with exact bit-width? I can wire simple validators accordingly.
src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
110-115: Confirm maxlength semantics vs bit widthIf inputs are binary strings, consider enforcing binary pattern and exact length; otherwise, maxlength may not reflect bit-width semantics.
Happy to wire validators once the expected value format is confirmed.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (2)
155-155: Compute tri-state results; avoid false negatives when results array is absent.When output.results is missing, you push false. Push null instead and update the type.
-const results: boolean[][][] = reactive([]); +const results = reactive<Array<Array<Array<boolean | null>>>>([]);- testBenchStore.testbenchData.testData.groups.map(group => { + testBenchStore.testbenchData.testData.groups.map(group => { results.push([]); group.outputs.map((output) => { results[results.length - 1].push([]); - for(let i = 0; i < output.values.length; i++) { - if(output.results && output.values[i] === output.results[i]) { - results[results.length - 1][results[results.length - 1].length - 1].push(true); - } else { - results[results.length - 1][results[results.length - 1].length - 1].push(false); - } - } + const hasResults = Array.isArray(output.results); + for (let i = 0; i < output.values.length; i++) { + const expected = hasResults ? output.results[i] : undefined; + if (expected === undefined) { + results[results.length - 1][results[results.length - 1].length - 1].push(null); + } else { + results[results.length - 1][results[results.length - 1].length - 1].push(output.values[i] === expected); + } + } }); });Also applies to: 211-228
243-276: Ensure bitWidth is numeric in payload.Prevents accidental string widths leaking to store.
- bitWidth: inputsBandWidth.value[index], + bitWidth: Number(inputsBandWidth.value[index]), ... - bitWidth: outputsBandWidth.value[index], + bitWidth: Number(outputsBandWidth.value[index]),
♻️ Duplicate comments (1)
v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
69-93: Fix bitwidth +/− controls: avoid string concatenation; enforce numeric model.Currently v-model binds strings and the “+” handler can turn 9 → 91 (string + number). Make the inputs numeric and coerce in handlers. Also aligns with your .no-spinner[type=number] CSS.
Apply:
- <v-btn icon size="x-small" variant="text" @click="inputsBandWidth[i] = Math.max(1, inputsBandWidth[i] - 1)" title="Decrease bitwidth"> + <v-btn icon size="x-small" variant="text" @click="inputsBandWidth[i] = Math.max(1, Number(inputsBandWidth[i]) - 1)" title="Decrease bitwidth"> <v-icon>mdi-minus</v-icon> </v-btn> - <input class="io-input bitwidth-input no-spinner" v-model="inputsBandWidth[i]" min="1" max="64" @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> - <v-btn icon size="x-small" variant="text" @click="inputsBandWidth[i] = Math.min(64, inputsBandWidth[i] + 1)" title="Increase bitwidth"> + <input class="io-input bitwidth-input no-spinner" type="number" step="1" + v-model.number="inputsBandWidth[i]" min="1" max="64" + @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> + <v-btn icon size="x-small" variant="text" @click="inputsBandWidth[i] = Math.min(64, Number(inputsBandWidth[i]) + 1)" title="Increase bitwidth"> <v-icon>mdi-plus</v-icon> </v-btn> ... - <v-btn icon size="x-small" variant="text" @click="outputsBandWidth[i] = Math.max(1, outputsBandWidth[i] - 1)" title="Decrease bitwidth"> + <v-btn icon size="x-small" variant="text" @click="outputsBandWidth[i] = Math.max(1, Number(outputsBandWidth[i]) - 1)" title="Decrease bitwidth"> <v-icon>mdi-minus</v-icon> </v-btn> - <input class="io-input bitwidth-input no-spinner" v-model="outputsBandWidth[i]" min="1" max="64" @blur="outputsBandWidth[i] = clamp(outputsBandWidth[i])"/> - <v-btn icon size="x-small" variant="text" @click="outputsBandWidth[i] = Math.min(64, outputsBandWidth[i] + 1)" title="Increase bitwidth"> + <input class="io-input bitwidth-input no-spinner" type="number" step="1" + v-model.number="outputsBandWidth[i]" min="1" max="64" + @blur="outputsBandWidth[i] = clamp(outputsBandWidth[i])"/> + <v-btn icon size="x-small" variant="text" @click="outputsBandWidth[i] = Math.min(64, Number(outputsBandWidth[i]) + 1)" title="Increase bitwidth"> <v-icon>mdi-plus</v-icon> </v-btn>
🧹 Nitpick comments (4)
v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (4)
164-169: Type the clamp helper and normalize inputs.Tighten types to reduce surprises; pairs well with v-model.number.
-const clamp = (val) => { - if (!val) return 1 - const n = Number(val) - if (isNaN(n)) return 1 - return Math.min(Math.max(n, 1), 64) -} +const clamp = (val: unknown): number => { + const n = Number(val); + if (!Number.isFinite(n)) return 1; + return Math.min(Math.max(n, 1), 64); +};
230-241: Avoid indexOf in loop; use forEach index.Removes O(n²) title updates and is clearer.
- if (testType.value === 'comb') { - groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); - } - else { - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); - } + const prefix = testType.value === 'comb' ? 'Group' : 'Set'; + groups.forEach((group, idx) => { group.title = `${prefix} ${idx + 1}`; });
379-404: CSV export: quote/escape fields.Names/titles with commas or quotes will break parsing. Quote and escape per RFC 4180.
Example:
const esc = (s: unknown) => `"${String(s).replace(/"/g, '""')}"`; let csv = `${esc(testType.value)},${esc(testTitle.value)}\n`; csv += `Inputs BandWidth: ${inputsBandWidth.value.join(',')}\n`; csv += `Outputs BandWidth: ${outputsBandWidth.value.join(',')}\n`; csv += `Inputs Name: ${inputsName.value.map(esc).join(',')}\n`; csv += `Outputs Name: ${outputsName.value.map(esc).join(',')}\n`;
413-438: CSV import: normalize line endings, trim, and strip trailing “:”.Makes import robust across OSes and prevents titles like “Group 1:” (with colon).
- const lines = csv.split('\n'); + const lines = csv.replace(/\r\n?/g, '\n').split('\n').map(l => l.trim()); ... - if (firstLine) { - [testType.value, testTitle.value] = firstLine.split(','); + if (firstLine) { + const [t, ...rest] = firstLine.split(','); + testType.value = (t || '').replace(/^"|"$/g, ''); + testTitle.value = rest.join(',').replace(/^"|"$/g, '').replace(/""/g, '"'); } ... - if (line.startsWith('G-')) { + if (line && line.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + const ci = line.indexOf(':'); + const title = ci >= 0 ? line.slice(2, ci) : line.slice(2); + group = { title, inputs: [], outputs: [] }; } else { - const [name, values] = line.split(':'); + if (!line) return; + const ci = line.indexOf(':'); + const name = ci >= 0 ? line.slice(0, ci) : line; + const values = ci >= 0 ? line.slice(ci + 1) : ''; const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); if (isInput) { group.inputs.push(values.split(',')); } else if (isOutput) { group.outputs.push(values.split(',')); } } }); - if (group.title) { + if (group.title) { newGroups.push(group); } groups.splice(0, groups.length, ...newGroups);Also applies to: 447-456, 451-465, 467-468
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue
- src/components/Panels/TestBenchPanel/TestBenchCreator.vue
Some UI/UX Suggestions
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
244-276: Send numeric, clamped bitwidths to the store.Guarantees schema correctness even if UI binds temporarily hold strings.
- bitWidth: inputsBandWidth.value[index], + bitWidth: clamp(inputsBandWidth.value[index]), @@ - bitWidth: outputsBandWidth.value[index], + bitWidth: clamp(outputsBandWidth.value[index]),src/components/Panels/TestBenchPanel/TestBenchCreator.vue (1)
244-276: Clamp bitwidths in payload.- bitWidth: inputsBandWidth.value[index], + bitWidth: clamp(inputsBandWidth.value[index]), @@ - bitWidth: outputsBandWidth.value[index], + bitWidth: clamp(outputsBandWidth.value[index]),
♻️ Duplicate comments (12)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (6)
279-291: Zero-fill new tests using current bitwidths (avoid width-1 placeholders).Push strings with the correct length to keep shapes consistent and prevent UI length mismatches.
const group = groups[index]; for (let i = 0; i < inputsBandWidth.value.length; i++) { if (group.inputs.length === i) group.inputs.push([]); - group.inputs[i].push("0"); + const wIn = Math.max(1, Number(inputsBandWidth.value[i]) || 1); + group.inputs[i].push("0".repeat(wIn)); } @@ for (let i = 0; i < outputsBandWidth.value.length; i++) { if (group.outputs.length === i) group.outputs.push([]); - group.outputs[i].push("0"); + const wOut = Math.max(1, Number(outputsBandWidth.value[i]) || 1); + group.outputs[i].push("0".repeat(wOut)); }
311-321: Zero-fill new IO columns to the correct width and count.When adding a new input/output, create a full column sized to current test count and bitwidth.
- group.inputs.push([]); - - for (let i = 0; i < group.inputs[0].length; i++) { - group.inputs[group.inputs.length - 1].push("0"); - } + const testCount = group.inputs[0]?.length ?? 0; + const width = Math.max(1, Number(inputsBandWidth.value.at(-1)) || 1); + group.inputs.push(Array.from({ length: testCount }, () => "0".repeat(width))); @@ - group.outputs.push([]); - - for (let i = 0; i < group.outputs[0].length; i++) { - group.outputs[group.outputs.length - 1].push("0"); - } + const testCount = group.outputs[0]?.length ?? 0; + const width = Math.max(1, Number(outputsBandWidth.value.at(-1)) || 1); + group.outputs.push(Array.from({ length: testCount }, () => "0".repeat(width)));Also applies to: 343-347
423-439: Harden CSV import: trim CRLF, sanitize widths to >=1, and strip trailing colon from group titles.Current parsing admits NaN/0 widths and leaves titles like “Group 1:”. Also guard empty/invalid lines.
- const lines = csv.split('\n'); + const lines = csv.split(/\r?\n/).map(l => l.trim()).filter(Boolean); @@ - if (line) { - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } + if (line) { + const bw = (line.split(': ')[1] || ''); + inputsBandWidth.value = bw.split(',').map(v => Math.max(1, parseInt(v, 10) || 1)); + } @@ - if (line) { - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); - } + if (line) { + const bw = (line.split(': ')[1] || ''); + outputsBandWidth.value = bw.split(',').map(v => Math.max(1, parseInt(v, 10) || 1)); + } @@ - if (line) { - inputsName.value = line.split(': ')[1].split(','); - } + if (line) { + inputsName.value = (line.split(': ')[1] || '').split(',').map(s => s.trim()).filter(Boolean); + } @@ - if (line) { - outputsName.value = line.split(': ')[1].split(','); - } + if (line) { + outputsName.value = (line.split(': ')[1] || '').split(',').map(s => s.trim()).filter(Boolean); + } @@ - if (line.startsWith('G-')) { + if (line.startsWith('G-')) { if (group.title) { newGroups.push(group); } - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { title: line.slice(2).replace(/:$/, '').trim(), inputs: [], outputs: [] }; } else { - const [name, values] = line.split(':'); + const sep = line.indexOf(':'); + if (sep === -1) return; + const name = line.slice(0, sep); + const values = line.slice(sep + 1); const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); if (isInput) { - group.inputs.push(values.split(',')); + group.inputs.push(values.split(',').map(s => s.trim())); } else if (isOutput) { - group.outputs.push(values.split(',')); + group.outputs.push(values.split(',').map(s => s.trim())); } }Also applies to: 447-468
119-128: Fix duplicated/unsafe results rendering (regression) — keep only the guarded block.Two overlapping v-for blocks render results; the first indexes unguarded and can crash when results aren’t hydrated. Remove it to avoid duplicate cells and runtime errors.
- <div v-if="testBenchStore.showResults" class="grid-cell results-col"> - <div v-for="(_, i) in results[groupIndex]" class="io-cell result-cell" :key="`g${groupIndex}-res-${i}-${testIndex}`" :class="results[groupIndex][i][testIndex] ? 'success' : 'fail'"> - <div - v-for="(_, i) in (results[groupIndex] || [])" - class="io-cell result-cell" - :key="`g${groupIndex}-res-${i}-${testIndex}`" - :class="(results[groupIndex]?.[i]?.[testIndex]) ? 'success' : 'fail'" - > - {{ (results[groupIndex]?.[i]?.[testIndex]) ? '✔' : '✘' }} - </div> - </div> + <div v-if="testBenchStore.showResults" class="grid-cell results-col"> + <div + v-for="(_, i) in (results[groupIndex] || [])" + class="io-cell result-cell" + :key="`g${groupIndex}-res-${i}-${testIndex}`" + :class="(results[groupIndex]?.[i]?.[testIndex]) ? 'success' : 'fail'"> + {{ (results[groupIndex]?.[i]?.[testIndex]) ? '✔' : '✘' }} + </div> + </div>
111-116: Remove TypeScript casts from template; ensure maxlength gets a real number.
as numberis not valid in Vue templates and can break compilation. Use clamp() (already defined) or helpers for numeric coercion.- <input class="io-input data-input" type="text" v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="inputsBandWidth[i] as number" /> + <input class="io-input data-input" type="text" v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="clamp(inputsBandWidth[i])" /> @@ - <input class="io-input data-input" type="text" v-model="group.outputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="outputsBandWidth[i]" /> + <input class="io-input data-input" type="text" v-model="group.outputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="clamp(outputsBandWidth[i])" />
73-81: Make bitwidth inputs numeric and keep model numeric.Use type="number" and v-model.number so widths don’t become strings and to align with clamp.
- <input class="io-input bitwidth-input no-spinner" v-model="inputsBandWidth[i]" min="1" max="64" @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> + <input class="io-input bitwidth-input no-spinner" type="number" step="1" + v-model.number="inputsBandWidth[i]" min="1" max="64" + @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> @@ - <input class="io-input bitwidth-input no-spinner" v-model="outputsBandWidth[i]" min="1" max="64" @blur="outputsBandWidth[i] = clamp(outputsBandWidth[i])"/> + <input class="io-input bitwidth-input no-spinner" type="number" step="1" + v-model.number="outputsBandWidth[i]" min="1" max="64" + @blur="outputsBandWidth[i] = clamp(outputsBandWidth[i])"/>Also applies to: 83-91
src/components/Panels/TestBenchPanel/TestBenchCreator.vue (6)
278-291: Zero-fill new tests using bitwidths.- group.inputs[i].push("0"); + group.inputs[i].push("0".repeat(Math.max(1, Number(inputsBandWidth.value[i]) || 1))); @@ - group.outputs[i].push("0"); + group.outputs[i].push("0".repeat(Math.max(1, Number(outputsBandWidth.value[i]) || 1)));
311-321: Zero-fill new IO columns with correct width and test count.- group.inputs.push([]); - for (let i = 0; i < group.inputs[0].length; i++) { - group.inputs[group.inputs.length - 1].push("0"); - } + const t = group.inputs[0]?.length ?? 0; + const w = Math.max(1, Number(inputsBandWidth.value.at(-1)) || 1); + group.inputs.push(Array.from({ length: t }, () => "0".repeat(w))); @@ - group.outputs.push([]); - for (let i = 0; i < group.outputs[0].length; i++) { - group.outputs[group.outputs.length - 1].push("0"); - } + const t = group.outputs[0]?.length ?? 0; + const w = Math.max(1, Number(outputsBandWidth.value.at(-1)) || 1); + group.outputs.push(Array.from({ length: t }, () => "0".repeat(w)));Also applies to: 344-347
423-439: Harden CSV import (CRLF, width clamp, title colon, robust splits).- const lines = csv.split('\n'); + const lines = csv.split(/\r?\n/).map(l => l.trim()).filter(Boolean); @@ - inputsBandWidth.value = line.split(': ')[1].split(',').map(Number); + inputsBandWidth.value = (line.split(': ')[1] || '').split(',').map(v => Math.max(1, parseInt(v, 10) || 1)); @@ - outputsBandWidth.value = line.split(': ')[1].split(',').map(Number); + outputsBandWidth.value = (line.split(': ')[1] || '').split(',').map(v => Math.max(1, parseInt(v, 10) || 1)); @@ - inputsName.value = line.split(': ')[1].split(','); + inputsName.value = (line.split(': ')[1] || '').split(',').map(s => s.trim()).filter(Boolean); @@ - outputsName.value = line.split(': ')[1].split(','); + outputsName.value = (line.split(': ')[1] || '').split(',').map(s => s.trim()).filter(Boolean); @@ - group = { title: line.slice(2), inputs: [], outputs: [] }; + group = { title: line.slice(2).replace(/:$/, '').trim(), inputs: [], outputs: [] }; @@ - const [name, values] = line.split(':'); + const sep = line.indexOf(':'); if (sep === -1) return; + const name = line.slice(0, sep); + const values = line.slice(sep + 1); @@ - group.inputs.push(values.split(',')); + group.inputs.push(values.split(',').map(s => s.trim())); @@ - group.outputs.push(values.split(',')); + group.outputs.push(values.split(',').map(s => s.trim()));Also applies to: 447-468
119-128: Fix duplicated/unsafe results rendering (regression) — keep only the guarded block.Same as v0: remove the unguarded v-for to prevent crashes and duplicate cells.
- <div v-for="(_, i) in results[groupIndex]" class="io-cell result-cell" :key="`g${groupIndex}-res-${i}-${testIndex}`" :class="results[groupIndex][i][testIndex] ? 'success' : 'fail'"> - <div - v-for="(_, i) in (results[groupIndex] || [])" + <div + v-for="(_, i) in (results[groupIndex] || [])" class="io-cell result-cell" :key="`g${groupIndex}-res-${i}-${testIndex}`" :class="(results[groupIndex]?.[i]?.[testIndex]) ? 'success' : 'fail'"> {{ (results[groupIndex]?.[i]?.[testIndex]) ? '✔' : '✘' }} </div>
110-116: Remove TypeScript casts from template; ensure maxlength gets a number.Use clamp() as the binding target.
- :maxlength="inputsBandWidth[i] as number" + :maxlength="clamp(inputsBandWidth[i])" @@ - :maxlength="outputsBandWidth[i]" + :maxlength="clamp(outputsBandWidth[i])"
72-81: Make bitwidth inputs numeric and typed.Enforce numeric model with type="number" and v-model.number.
- <input class="io-input bitwidth-input no-spinner" v-model="inputsBandWidth[i]" min="1" max="64" @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> + <input class="io-input bitwidth-input no-spinner" type="number" step="1" + v-model.number="inputsBandWidth[i]" min="1" max="64" + @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> @@ - <input class="io-input bitwidth-input no-spinner" v-model="outputsBandWidth[i]" min="1" max="64" @blur="outputsBandWidth[i] = clamp(outputsBandWidth[i])"/> + <input class="io-input bitwidth-input no-spinner" type="number" step="1" + v-model.number="outputsBandWidth[i]" min="1" max="64" + @blur="outputsBandWidth[i] = clamp(outputsBandWidth[i])"/>Also applies to: 83-91
🧹 Nitpick comments (4)
v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (2)
230-241: Use index param instead of indexOf in watcher.Avoid O(n²) and wrong indices when titles repeat.
- if (testType.value === 'comb') { - groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); - } - else { - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); - } + const make = testType.value === 'comb' ? 'Group' : 'Set'; + groups.forEach((group, idx) => { group.title = `${make} ${idx + 1}`; });
109-117: Optional UX: 0/1 toggles for data cells to match review feedback.Replace text inputs with a compact toggle that flips between '0' and '1' on click; keeps maxlength logic implicit.
- <input class="io-input data-input" type="text" v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="clamp(inputsBandWidth[i])" /> + <v-btn size="x-small" variant="text" + :disabled="testBenchStore.readOnly" + @click="group.inputs[i][testIndex] = group.inputs[i][testIndex] === '1' ? '0' : '1'"> + {{ group.inputs[i][testIndex] ?? '0' }} + </v-btn>Apply similarly for outputs if desired.
src/components/Panels/TestBenchPanel/TestBenchCreator.vue (2)
230-241: Title watcher micro-opt.- if (testType.value === 'comb') { - groups.forEach(group => { - group.title = `Group ${groups.indexOf(group) + 1}`; - }); - } - else { - groups.forEach(group => { - group.title = `Set ${groups.indexOf(group) + 1}`; - }); - } + const prefix = testType.value === 'comb' ? 'Group' : 'Set'; + groups.forEach((g, i) => { g.title = `${prefix} ${i + 1}`; });
108-116: Optional: 0/1 toggles for IO cells to match UX feedback.- <input class="io-input data-input" type="text" v-model="group.outputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="clamp(outputsBandWidth[i])" /> + <v-btn size="x-small" variant="text" + :disabled="testBenchStore.readOnly" + @click="group.outputs[i][testIndex] = group.outputs[i][testIndex] === '1' ? '0' : '1'"> + {{ group.outputs[i][testIndex] ?? '0' }} + </v-btn>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)v0/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue
niladrix719
left a comment
niladrix719
left a comment
There was a problem hiding this comment.
can you fix the extra div
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (8)
v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (4)
156-156: Tri-state results model to support neutral state and avoid false negatives.Use (boolean | null) to represent unknown results; compute null when output.results missing or index out of range.
-const results: boolean[][][] = reactive([]); +const results = reactive<(boolean | null)[][][]>([]);- testBenchStore.testbenchData.testData.groups.map(group => { + testBenchStore.testbenchData.testData.groups.map(group => { results.push([]); group.outputs.map((output) => { results[results.length - 1].push([]); - for(let i = 0; i < output.values.length; i++) { - if(output.results && output.values[i] === output.results[i]) { - results[results.length - 1][results[results.length - 1].length - 1].push(true); - } else { - results[results.length - 1][results[results.length - 1].length - 1].push(false); - } - } + for (let i = 0; i < output.values.length; i++) { + const expected = output.results?.[i]; + if (expected == null) { + results[results.length - 1][results[results.length - 1].length - 1].push(null); + } else { + results[results.length - 1][results[results.length - 1].length - 1].push(output.values[i] === expected); + } + } }); });Also applies to: 212-229
111-116: Remove TS assertion in template; coerce maxlength numerically.Type assertions don’t exist at runtime. Convert to Number().
-<input class="io-input data-input" type="text" v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="inputsBandWidth[i] as number" /> +<input class="io-input data-input" type="text" v-model="group.inputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="Number(inputsBandWidth[i]) || 0" /> -<input class="io-input data-input" type="text" v-model="group.outputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="outputsBandWidth[i]" /> +<input class="io-input data-input" type="text" v-model="group.outputs[i][testIndex]" :disabled="testBenchStore.readOnly" :maxlength="Number(outputsBandWidth[i]) || 0" />
119-129: Fix duplicated results loop and unsafe indexing; add neutral state.There are two nested v-for blocks in the results column; the first directly indexes results[groupIndex][i][testIndex] and can crash. It also duplicates cells. Replace with a single guarded loop and render a neutral state when value is undefined.
- <div v-if="testBenchStore.showResults" class="grid-cell results-col"> - <div v-for="(_, i) in results[groupIndex]" class="io-cell result-cell" :key="`g${groupIndex}-res-${i}-${testIndex}`" :class="results[groupIndex][i][testIndex] ? 'success' : 'fail'"> - <div - v-for="(_, i) in (results[groupIndex] || [])" - class="io-cell result-cell" - :key="`g${groupIndex}-res-${i}-${testIndex}`" - :class="(results[groupIndex]?.[i]?.[testIndex]) ? 'success' : 'fail'" - > - {{ (results[groupIndex]?.[i]?.[testIndex]) ? '✔' : '✘' }} - </div> - </div> + <div v-if="testBenchStore.showResults" class="grid-cell results-col"> + <div + v-for="(_, i) in (results[groupIndex] || [])" + class="io-cell result-cell" + :key="`g${groupIndex}-res-${i}-${testIndex}`" + :class="{ + success: results[groupIndex]?.[i]?.[testIndex] === true, + fail: results[groupIndex]?.[i]?.[testIndex] === false, + neutral: results[groupIndex]?.[i]?.[testIndex] == null + }" + > + {{ results[groupIndex]?.[i]?.[testIndex] === true + ? '✔' + : (results[groupIndex]?.[i]?.[testIndex] === false ? '✘' : '—') }} + </div> + </div>Add style:
.result-cell.fail { color: var(--cv-red); } + +.result-cell.neutral { + color: var(--cv-text-secondary); +}
74-81: Make bitwidth inputs numeric and robust.Use number inputs with v-model.number; coerce in +/- handlers to avoid string math.
- <v-btn icon size="x-small" variant="text" @click="inputsBandWidth[i] = Math.max(1, inputsBandWidth[i] - 1)" title="Decrease bitwidth"> + <v-btn icon size="x-small" variant="text" @click="inputsBandWidth[i] = Math.max(1, Number(inputsBandWidth[i]) - 1)" title="Decrease bitwidth"> ... - <input class="io-input bitwidth-input no-spinner" v-model="inputsBandWidth[i]" min="1" max="64" @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> + <input class="io-input bitwidth-input no-spinner" type="number" step="1" + v-model.number="inputsBandWidth[i]" min="1" max="64" + @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> ... - <v-btn icon size="x-small" variant="text" @click="inputsBandWidth[i] = Math.min(64, inputsBandWidth[i] + 1)" title="Increase bitwidth"> + <v-btn icon size="x-small" variant="text" @click="inputsBandWidth[i] = Math.min(64, Number(inputsBandWidth[i]) + 1)" title="Increase bitwidth">Repeat the same changes for outputsBandWidth.
Also applies to: 85-91
src/components/Panels/TestBenchPanel/TestBenchCreator.vue (4)
156-156: Tri-state results model to support neutral state and avoid false negatives.Mirror the v1 change: (boolean | null)[][][] and null for unknown.
-const results: boolean[][][] = reactive([]); +const results = reactive<(boolean | null)[][][]>([]);- for(let i = 0; i < output.values.length; i++) { - if(output.results && output.values[i] === output.results[i]) { - results[results.length - 1][results[results.length - 1].length - 1].push(true); - } else { - results[results.length - 1][results[results.length - 1].length - 1].push(false); - } - } + for (let i = 0; i < output.values.length; i++) { + const expected = output.results?.[i]; + if (expected == null) { + results[results.length - 1][results[results.length - 1].length - 1].push(null); + } else { + results[results.length - 1][results[results.length - 1].length - 1].push(output.values[i] === expected); + } + }Also applies to: 212-229
111-116: Remove TS assertion in template; coerce maxlength numerically.Same as v1.
-:maxlength="inputsBandWidth[i] as number" +:maxlength="Number(inputsBandWidth[i]) || 0" ... -:maxlength="outputsBandWidth[i]" +:maxlength="Number(outputsBandWidth[i]) || 0"
119-129: Fix duplicated results loop and unsafe indexing; add neutral state.Same issue as v1 file; apply identical fix.
- <div v-if="testBenchStore.showResults" class="grid-cell results-col"> - <div v-for="(_, i) in results[groupIndex]" class="io-cell result-cell" :key="`g${groupIndex}-res-${i}-${testIndex}`" :class="results[groupIndex][i][testIndex] ? 'success' : 'fail'"> - <div - v-for="(_, i) in (results[groupIndex] || [])" - class="io-cell result-cell" - :key="`g${groupIndex}-res-${i}-${testIndex}`" - :class="(results[groupIndex]?.[i]?.[testIndex]) ? 'success' : 'fail'" - > - {{ (results[groupIndex]?.[i]?.[testIndex]) ? '✔' : '✘' }} - </div> - </div> + <div v-if="testBenchStore.showResults" class="grid-cell results-col"> + <div + v-for="(_, i) in (results[groupIndex] || [])" + class="io-cell result-cell" + :key="`g${groupIndex}-res-${i}-${testIndex}`" + :class="{ + success: results[groupIndex]?.[i]?.[testIndex] === true, + fail: results[groupIndex]?.[i]?.[testIndex] === false, + neutral: results[groupIndex]?.[i]?.[testIndex] == null + }" + > + {{ results[groupIndex]?.[i]?.[testIndex] === true + ? '✔' + : (results[groupIndex]?.[i]?.[testIndex] === false ? '✘' : '—') }} + </div> + </div>Add style:
.result-cell.fail { color: var(--cv-red); } + +.result-cell.neutral { + color: var(--cv-text-secondary); +}
74-81: Make bitwidth inputs numeric and robust.Same as v1.
- <input class="io-input bitwidth-input no-spinner" v-model="inputsBandWidth[i]" min="1" max="64" @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/> + <input class="io-input bitwidth-input no-spinner" type="number" step="1" + v-model.number="inputsBandWidth[i]" min="1" max="64" + @blur="inputsBandWidth[i] = clamp(inputsBandWidth[i])"/>Repeat for outputsBandWidth and coerce in +/- handlers with Number(...).
Also applies to: 85-91
🧹 Nitpick comments (10)
v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue (5)
165-170: Type clamp and harden numeric conversion.Prevents NaN and fractional widths.
-const clamp = (val) => { - if (!val) return 1 - const n = Number(val) - if (isNaN(n)) return 1 - return Math.min(Math.max(n, 1), 64) -} +const clamp = (val: unknown): number => { + const n = Number(val); + if (!Number.isFinite(n)) return 1; + return Math.min(64, Math.max(1, Math.trunc(n))); +};
244-268: Ensure bitWidth fields are numbers in payload.Avoid leaking strings to store/validator.
- bitWidth: inputsBandWidth.value[index], + bitWidth: Number(inputsBandWidth.value[index]), ... - bitWidth: outputsBandWidth.value[index], + bitWidth: Number(outputsBandWidth.value[index]),
447-462: Guard CSV parsing and split only on first colon.Prevents crashes on malformed lines and titles/names containing colons.
- } else { - const [name, values] = line.split(':'); - const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); - const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); - if (isInput) { - group.inputs.push(values.split(',')); - } else if (isOutput) { - group.outputs.push(values.split(',')); - } - } + } else if (line.trim().length) { + const idx = line.indexOf(':'); + if (idx === -1) return; + const name = line.slice(0, idx); + const valuesStr = line.slice(idx + 1); + const isInput = name.startsWith('I-') && inputsName.value.includes(name.slice(2)); + const isOutput = name.startsWith('O-') && outputsName.value.includes(name.slice(2)); + if (isInput) { + group.inputs.push(valuesStr.split(',')); + } else if (isOutput) { + group.outputs.push(valuesStr.split(',')); + } + }
35-47: Minor alignment nit: keep header, labels, and bitwidth grids consistent.Add the same padding/border styles to header-grid to eliminate top-row misalignment noted in review.
-.data-grid.header-grid { /* inherits */ } +.data-grid.header-grid { + align-items: center; +}If misalignment persists, consider equalizing row heights with min-height on .inputs-col/.outputs-col across grids.
Also applies to: 52-68, 70-94
110-117: Optional: 0/1 toggles for single-bit inputs as suggested by design.When corresponding bitwidth is 1, render a binary toggle instead of free-text to reduce errors.
I can draft a small ToggleCell that binds to group.inputs[i][testIndex] and flips between '0' and '1'.
src/components/Panels/TestBenchPanel/TestBenchCreator.vue (5)
165-170: Type clamp and harden numeric conversion.Same as v1.
-const clamp = (val) => { +const clamp = (val: unknown): number => {
244-268: Ensure bitWidth fields are numbers in payload.Same as v1.
- bitWidth: inputsBandWidth.value[index], + bitWidth: Number(inputsBandWidth.value[index]), ... - bitWidth: outputsBandWidth.value[index], + bitWidth: Number(outputsBandWidth.value[index]),
447-462: Guard CSV parsing and split only on first colon.Same as v1.
-const [name, values] = line.split(':'); +const idx = line.indexOf(':'); +if (idx === -1) return; +const name = line.slice(0, idx); +const valuesStr = line.slice(idx + 1); ... - group.inputs.push(values.split(',')); + group.inputs.push(valuesStr.split(','));
35-47: Minor alignment nit: keep header, labels, and bitwidth grids consistent.Same as v1.
Also applies to: 52-68, 70-94
110-117: Optional: 0/1 toggles for single-bit inputs as suggested by design.Same as v1.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)v1/src/components/Panels/TestBenchPanel/TestBenchCreator.vue(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
2 participants
Fixes #651
Previews
Implementation
Testbench.mp4
Summary by CodeRabbit
New Features
Improvements
Style