Variable popover fix

[johbaxter]

Description

Changes Made

How to Test

1. Steps to reproduce/test the behavior
2. Expected outcomes

Notes

[github-actions]

@CodiumAI-Agent /describe

[QodoAI-Agent]

Title

Variable popover fix

---

User description

Description

Changes Made

How to Test

1. Steps to reproduce/test the behavior
2. Expected outcomes

Notes

---

PR Type

Enhancement

---

Description

• Normalize imports quoting and ordering

• Standardize component indentation and spacing

• Simplify comparisonBlocks to empty array

• Add TODO for dead code removal

---

File Walkthrough

	Relevant files
Formatting	AddVariablePopover.tsx Refactor imports & remove dead code                                            packages/client/src/components/notebook/AddVariablePopover.tsx Reformatted imports to double quotes and grouped logically Adjusted indentation and component code style Simplified comparisonBlocks to always return empty array Added TODO comment to remove dead code +887/-904

---

[github-actions]

@CodiumAI-Agent /review

[github-actions]

@CodiumAI-Agent /improve

[QodoAI-Agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Engine Type The inline state type for engine has an untyped app_subtype property, which may cause TypeScript errors and runtime inconsistencies. Ensure app_subtype is defined with a proper type. const [engine, setEngine] = useState<{ app_id: string; app_name: string; app_type: string; app_subtype; } | null>(null); Select Value Passing the entire engine object to Select.Item value may lead to unexpected behavior since value typically expects a primitive. Consider using app_id as the value and mapping back to the object on change. return engines.models.map((model) => { return ( <Select.Item key={model.app_id} value={model}> <Typography variant="caption"> {model.app_name} </Typography> </Select.Item> ); Dead Code The comparisonBlocks computed returns an empty array and contains a TODO to remove dead code. Clean up or implement the intended logic to keep the codebase maintainable. const comparisonBlocks = computed(() => { return []; }).get();

[QodoAI-Agent]

PR Code Suggestions ✨

Category	Suggestion	Impact
Possible issue	Reinstate LLM comparison filtering The comparisonBlocks array is stubbed out and will never list any blocks for "LLM Comparison" variables. Restore the original filter logic so that it actually collects blocks of type BLOCK_TYPE_COMPARE from state.blocks. packages/client/src/components/notebook/AddVariablePopover.tsx [191-195] // Get the LLM Comparison Blocks -// TODO: REMOVE DEAD CODE const comparisonBlocks = computed(() => { - return []; + return Object.values(state.blocks) + .filter( + (block) => DefaultBlocks[block.widget].type === BLOCK_TYPE_COMPARE, + ) + .sort((a, b) => { + const aId = a.id.toLowerCase(), bId = b.id.toLowerCase(); + if (aId < bId) return -1; + if (aId > bId) return 1; + return 0; + }); }).get(); Suggestion importance[1-10]: 8 __ Why: The stubbed comparisonBlocks always returns an empty array, so restoring the filter against BLOCK_TYPE_COMPARE is critical to populate LLM comparison options and preserve intended functionality.	Medium
	Fix missing app_subtype type The inline type for engine is missing a type annotation for app_subtype, causing a TypeScript syntax error. Add : string to the app_subtype field to fix the type definition. packages/client/src/components/notebook/AddVariablePopover.tsx [130-135] const [engine, setEngine] = useState<{ app_id: string; app_name: string; app_type: string; - app_subtype; + app_subtype: string; } | null>(null); Suggestion importance[1-10]: 8 __ Why: The app_subtype property has no type annotation, causing a TypeScript syntax error; adding : string fixes the type definition and ensures the code compiles.	Medium
General	Use primitive values in selects Passing the entire model object as the value of each <Select.Item> can lead to unexpected behavior; it’s safer to use a primitive key (like model.app_id) and then look up the object on change. packages/client/src/components/notebook/AddVariablePopover.tsx [240-248] engines.models.map((model) => { return ( - <Select.Item key={model.app_id} value={model}> + <Select.Item key={model.app_id} value={model.app_id}> <Typography variant="caption">{model.app_name}</Typography> </Select.Item> ); }); Suggestion importance[1-10]: 5 __ Why: Passing the entire model object as the <Select.Item> value can lead to identity and comparison issues; using the primitive model.app_id ensures consistent and predictable select behavior.	Low

[github-actions]

@CodiumAI-Agent /update_changelog