Skip to content

feat(libs): query settings input tool enhancement#722

Closed
Paulson-Robert wants to merge 12 commits intodevfrom
638-Query-input-settings-enhacement
Closed

feat(libs): query settings input tool enhancement#722
Paulson-Robert wants to merge 12 commits intodevfrom
638-Query-input-settings-enhacement

Conversation

@Paulson-Robert
Copy link
Copy Markdown
Contributor

Description

Changes Made

How to Test

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

Notes

@Paulson-Robert Paulson-Robert requested a review from a team as a code owner March 18, 2025 13:16
@Paulson-Robert Paulson-Robert self-assigned this Mar 18, 2025
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

feat(libs): query settings input tool enhancement


User description

Description

Changes Made

How to Test

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

Notes


PR Type

  • Enhancement

Description

  • Added new AddVariable SVG icon component.

  • Extended QueryInputSettings with new options interface.

  • Implemented styled accordion for grouping options.

  • Integrated variable addition with notifications.


Changes walkthrough 📝

Relevant files
Enhancement
AddVariable.tsx
New SVG icon component for variables                                         

libs/renderer/src/assets/AddVariable.tsx

  • New SVG icon component added.
  • Supports customizable color, width, height.
  • +36/-0   
    QueryInputSettings.tsx
    Enhanced QueryInputSettings with grouping and notifications

    libs/renderer/src/components/block-settings/custom/QueryInputSettings.tsx

  • Extended Option interface with new properties.
  • Added styled accordion for grouping options.
  • Integrated handleVariablize with notifications.
  • Refactored Autocomplete filtering and rendering.
  • +331/-28

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /review

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Complexity

    The new option mapping and filtering logic includes multiple nested conditionals and iterative loops, which could hurt readability and maintainability. Consider refactoring parts of it into separate helper functions.

         */
        display: string;
    
        /**
         * type of block
         */
        blockType: "block" | "query" | "cell" | "query-prop" | "cell-prop" | "cell";
    
        /**
         * whether the option is variabilized
         * @type {boolean}
         * @default false
         */
        variabilized: boolean;
    
        /**
         * Group alias for grouping options
         * @type {string}
         * @default ""
         */
        groupAlias: string;
    }
    
    const StyledModalHeader = styled(Stack)(({ theme }) => ({
        padding: theme.spacing(2),
        flexDirection: "row",
        justifyContent: "space-between",
        alignItems: "center",
    }));
    
    const StyledMenuSection = styled(Accordion)(({ theme }) => ({
        boxShadow: "none",
        borderRadius: "0 !important",
        border: "0px",
        borderBottom: `1px solid ${theme.palette.divider}`,
        "&:before": {
            display: "none",
        },
        "&.Mui-expanded": {
            margin: "0",
            "&:last-child": {
                borderBottom: "0px",
            },
        },
    }));
    
    const StyledMenuSectionTitle = styled(Accordion.Trigger)(({ theme }) => ({
        minHeight: "auto !important",
        height: theme.spacing(6),
    }));
    
    // Group name mapper function
    const groupAliasMapper = (type: string) => {
        switch (type) {
            case "query":
                return "Notebook";
            case "cell":
                return "Cell";
            case "cell-prop":
                return "Cell-prop";
            case "block":
                return "Block";
            case "query-prop":
                return "Notebook-prop";
            default:
                return type.charAt(0).toUpperCase() + type.slice(1);
        }
    };
    
    /**
     * Specifically for selecting a query for to associate with a UI block
     */
    export const QueryInputSettings = observer(
        <D extends BlockDef = BlockDef>({
            id,
            path,
            label,
        }: QueryInputSettingsProps<D>) => {
            const { data, setData } = useBlockSettings(id);
            const { state, notebook } = useBlocks();
            const notification = useNotification();
    
            // track the value
            const [value, setValue] = useState("");
            // internal state of the input component
            const [inputValue, setInputValue] = useState("");
            // track the modal
            const [open, setOpen] = useState(false);
            // Track the input ref to grab the cursor position
            const inputRef = useRef(null);
            const suggestionRef = useRef(null);
            const measureRef = useRef(null);
            // track the ref to debounce the input
            const timeoutRef = useRef<ReturnType<typeof setTimeout>>(null);
    
            // get the value of the input (wrapped in usememo because of path prop)
            const computedValue = useMemo(() => {
                return computed(() => {
                    if (!data) {
                        return "";
                    }
    
                    const v = getValueByPath(data, path);
    
                    if (typeof v === "undefined") {
                        return "";
                    } else if (typeof v === "string") {
                        return v;
                    }
    
                    return JSON.stringify(v);
                });
            }, [data, path]).get();
    
            // update the value whenever the computed one changes
            useEffect(() => {
                setValue(computedValue);
                setInputValue(computedValue);
            }, [computedValue]);
    
            /**
             * Sync the data on change
             */
            const onChange = (value: string) => {
                // set the value
                setValue(value);
    
                // clear out the old timeout
                if (timeoutRef.current) {
                    clearTimeout(timeoutRef.current);
                    timeoutRef.current = null;
                }
    
                timeoutRef.current = setTimeout(() => {
                    try {
                        setData(path, value as PathValue<D["data"], typeof path>);
                    } catch (e) {
                        console.log(e);
                    }
                }, 300);
            };
    
            const optionMap = useMemo<Record<string, Option>>(() => {
                const pathMap = {};
                const variabilizedList = [];
    
                // iterate over the variables
                Object.entries(state.variables).forEach(
                    (keyValue: [string, Variable]) => {
                        const alias = keyValue[0];
                        const variable = keyValue[1];
    
                        const ref = state.getVariable(variable.to, variable.type);
    
                        // check if the variable is variabilized
                        if (
                            variable.type === "block" &&
                            !variabilizedList.includes(variable.to)
                        )
                            variabilizedList.push(variable.to);
                        else if (
                            variable.type === "cell" &&
                            !variabilizedList.includes(variable.cellId)
                        )
                            variabilizedList.push(variable.cellId);
                        else if (
                            variable.type === "query" &&
                            !variabilizedList.includes(variable.to)
                        )
                            variabilizedList.push(variable.to);
    
                        pathMap[alias] = {
                            id: alias,
                            path: alias,
                            type: typeof ref,
                            display: alias,
                            blockType: variable.type,
                            variabilized: true,
                            groupAlias: groupAliasMapper(variable.type),
                        };
    
                        if (variable.type === "query") {
                            const q = state.getQuery(variable.to);
                            for (const f in q._exposed) {
                                pathMap[`${alias}.${f}`] = {
                                    id: `${alias}.${f}`,
                                    path: `${alias}.${f}`,
                                    type: typeof q[f], // TODO: get value
                                    display: `${alias}.${f}`,
                                    blockType: "query-prop",
                                    variabilized: true,
                                    groupAlias: groupAliasMapper("query-prop"),
                                };
                            }
                        }
    
                        if (variable.type === "cell") {
                            const q = state.getQuery(variable.to);
                            const c = q.getCell(variable.cellId);
    
                            for (const f in c._exposed) {
                                pathMap[`${alias}.${f}`] = {
                                    id: `${alias}.${f}`,
                                    path: `${alias}.${f}`,
                                    type: typeof c[f], // TODO: get value
                                    display: `${alias}.${f}`,
                                    blockType: "cell-prop",
                                    variabilized: true,
                                    groupAlias: groupAliasMapper("cell-prop"),
                                };
                            }
                        }
                    },
                );
    
                // iterate over the blocks
                Object.entries(state.blocks).forEach(
                    (keyValue: [string, Block]) => {
                        const alias = keyValue[0];
                        const block = keyValue[1];
                        //filter only valid(variabilizable) blocks
                        if (
                            INPUT_BLOCK_TYPES.indexOf(block.widget) > -1 &&
                            !variabilizedList.includes(alias)
                        ) {
                            pathMap[alias] = {
                                id: alias,
                                path: alias,
                                type: typeof block,
                                display: alias,
                                blockType: "block",
                                variabilized: Object.keys(state.variables).includes(
                                    alias,
                                ),
                                groupAlias: groupAliasMapper("block"),
                            };
                        }
                    },
                );
    
                // iterate over the Queries
                Object.entries(state.queries).forEach(
                    (keyValue: [string, QueryState]) => {
                        const alias = keyValue[0];
                        const query = keyValue[1];
    
                        if (!variabilizedList.includes(alias)) {
                            pathMap[alias] = {
                                id: alias,
                                path: alias,
                                type: typeof query,
                                display: alias,
                                blockType: "query",
                                variabilized: Object.keys(state.variables).includes(
                                    alias,
                                ),
                                groupAlias: groupAliasMapper("query"),
                            };
    
                            const q = state.getQuery(alias);
                            for (const f in q._exposed) {
                                pathMap[`${alias}.${f}`] = {
                                    id: `${alias}.${f}`,
                                    path: `${alias}.${f}`,
                                    type: typeof q[f], // TODO: get value
                                    display: `${alias}.${f}`,
                                    blockType: "query-prop",
                                    variabilized: true,
                                    groupAlias: groupAliasMapper("query-prop"),
                                };
    Robustness

    The added handleVariablize function relies on splitting the option path to derive variable identifiers, which may be brittle if the data format changes. It would be beneficial to add error handling or validations for unexpected path formats.

    /**
     * @name handleVariablize
     * Adds a new variable to the state
     */
    const handleVariablize = (option: Option) => {
        // add variable
        const success = state.dispatch({
            message: ActionMessages.ADD_VARIABLE,
            payload: {
                id:
                    option.blockType === "cell"
                        ? option?.path?.split(".")[1]
                        : option.id,
                to:
                    option.blockType === "cell"
                        ? option?.path?.split(".")[0]
                        : option?.path,
                cellId:
                    option.blockType === "cell"
                        ? option?.path?.split(".")[1]
                        : null,
                type: option.blockType as VariableType,
            },
        });
    
        // Create notification
        notification.add({
            color: success ? "success" : "error",
            message: success
                ? `Successfully added ${option.id} as a variable.`
                : `Unable to add ${option.id}, due to syntax or a duplicated alias`,
        });
    };

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @johbaxter
    Copy link
    Copy Markdown
    Contributor

    #750

    Merged into here

    @johbaxter johbaxter closed this Mar 25, 2025
    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    QueryInputSettings - show all blocks/notebooks/cells in list even if not variable

    3 participants