Skip to content

feat(client/renderer): create notebook variables tie to community block#1433

Merged
johbaxter merged 30 commits intodevfrom
1340-Create-Notebook-Community-Block-Tie
Jul 31, 2025
Merged

feat(client/renderer): create notebook variables tie to community block#1433
johbaxter merged 30 commits intodevfrom
1340-Create-Notebook-Community-Block-Tie

Conversation

@Paulson-Robert
Copy link
Copy Markdown
Contributor

@Paulson-Robert Paulson-Robert commented Jul 7, 2025

…cks FE implementation

Description

Right now, our community block flow is reusable UI blocks that get saved into the database. However, we want reusable components in general, not just reusable UI components.

The goal is to make a reusable form. This is just for POC. There will be a singular notebook dependency tied into the UI.

On this implementation, the dependency variables and notebook/query will brought in along with the community blocks drops.

Changes Made

  1. Before saving the community block, written a scanner function to identify the dependent variable and queries to the block and save that as well.
  2. On drop area before generating the desired ui block, trying generate all the dependent quires and variables required and update the block json with the newly generated dependent item's id.
  3. Added isCommunity flag to the schema to refer a particular block is a community block or not. This flag is also used to isolate the added logic specific to community block.

How to Test

  1. Create a community block with some dependent variables and queries in it.
  2. Drag and drop from the community bocks tab, you can notice in notebook and variables panel with some auto generated required variables and notebooks.
SEMOSS.2.webm

Notes

@Paulson-Robert Paulson-Robert requested a review from johbaxter July 7, 2025 05:42
@Paulson-Robert Paulson-Robert self-assigned this Jul 7, 2025
@Paulson-Robert Paulson-Robert requested a review from a team as a code owner July 7, 2025 05:42
@Paulson-Robert Paulson-Robert linked an issue Jul 7, 2025 that may be closed by this pull request
1 task
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 7, 2025

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

feat(client/renderer): create notebook variables tie to community block


User description

…cks FE implementation

Description

Changes Made

How to Test

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

Notes


PR Type

Enhancement


Description

  • Track community block referenceId in state store

  • Clone and rewrite query IDs before adding block

  • Dispatch dependency queries and handle naming conflicts

  • Dispatch dependency variables with updated IDs


Changes diagram

flowchart LR
  A["Select community block"] -- "prepare dependencies" --> B["dispatchDependencyQueries"]
  B -- "ADD_BLOCK with rewritten JSON" --> C["state.dispatch ADD_BLOCK"]
  C -- "dispatchDependencyVariables" --> D["state.dispatch ADD_VARIABLE"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
state.store.ts
Save referenceId for community blocks                                       

libs/renderer/src/store/state/state.store.ts

  • Save json.id as block.referenceId
  • Add conditional assignment for community blocks
  • +5/-0     
    state.types.ts
    Extend block types with referenceId and id                             

    libs/renderer/src/store/state/state.types.ts

  • Add optional referenceId to Block type
  • Add optional id to BlockJSON for source blocks
  • +5/-0     
    AddBlocksMenuCard.tsx
    Handle block query & variable dependencies                             

    packages/client/src/components/designer/AddBlocksMenuCard.tsx

  • Import QueryStateConfig, Variable, VariableWithId
  • Add rewriteQueryIdInBlocksJSON clone/replace helper
  • Implement dispatchDependencyQueries and dispatchDependencyVariables
  • Update block add/replace flow to handle queries & variables
  • +399/-3 
    AddClientBlockModal.tsx
    Scan and include block queries/variables                                 

    packages/client/src/components/designer/AddClientBlockModal.tsx

  • Include id in allowed keys for slot processing
  • Add scanBlocks to collect queries & variables
  • Extend handleAddAsClientBlock to attach dependencies
  • +113/-2 

    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

    github-actions bot commented Jul 7, 2025

    @CodiumAI-Agent /review

    @github-actions
    Copy link
    Copy Markdown

    github-actions bot commented Jul 7, 2025

    @CodiumAI-Agent /improve

    @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

    Missing block_json

    newClientBlock is constructed but never passed to the AddBlock request. Update the runQuery call to include a serialized block_json payload.

    let newClientBlock = {
        widget: block.widget,
        data: block.data,
        listeners: block.listeners,
        slots: processSlots(block.slots, state.blocks),
        id: block.id,
    };
    const result = scanBlocks(
        newClientBlock,
        state.queries,
        state.variables,
    );
    newClientBlock = {
        ...newClientBlock,
        queries: result.queries,
        variables: result.variables,
    } as typeof newClientBlock & { queries: Dict; variables: Dict };
    Deep recursion

    The rewriteQueryIdInBlocksJSON helper uses deep recursive cloning and regex replacements, which can lead to performance bottlenecks or stack overflows on large JSON inputs. Consider an iterative or streaming approach.

    function rewriteQueryIdInBlocksJSON<T>(
        input: T,
        target: string,
        replacement: string,
        skipKeys: Set<string> = new Set(['variables', 'queries']),
    ): T {
        const mustacheRE = new RegExp(
            `{{\\s*(${target})(\\.[^{}\\s]*)?\\s*}}`,
            'g',
        );
    
        function cloneAndReplace(node: any): any {
            if (node == null) return node;
    
            /* -------- strings -------- */
            if (typeof node === 'string') {
                if (node === target) return replacement; // exact match
                return node.replace(
                    mustacheRE,
                    (_, __, rest = '') => `{{${replacement}${rest}}}`,
                ); // moustache
            }
    
            /* -------- arrays --------- */
            if (Array.isArray(node)) {
                return node.map(cloneAndReplace);
            }
    
            /* -------- objects -------- */
            if (typeof node === 'object') {
                const result: any = {};
                for (const [key, value] of Object.entries(node)) {
                    /* Skip entire subtree if key is in skipKeys */
                    if (skipKeys.has(key)) {
                        result[key] = value; // shallow copy is fine – untouched
                        continue;
                    }
    
                    /* Replace fields like { queryId: "form-submit" } */
                    if (key === 'queryId' && value === target) {
                        result[key] = replacement;
                    } else {
                        result[key] = cloneAndReplace(value);
                    }
                }
                return result;
            }
    
            /* -------- primitives (number, boolean, etc.) -------- */
            return node;
        }
    
        return cloneAndReplace(input);
    }
    Query scanning logic

    In scanBlocks, any object id field is treated as a query ID, potentially misclassifying block IDs. Refine the detection to only capture actual query definitions.

    // RUN_QUERY or bare { queryId: "…" }
    const maybeId =
        (node as any).payload?.queryId ??
        (node as any).queryId ??
        (node as any).id; // <- adjust if your schema differs
    if (typeof maybeId === 'string') qIds.add(maybeId);

    @QodoAI-Agent
    Copy link
    Copy Markdown

    QodoAI-Agent commented Jul 7, 2025

    PR Code Suggestions ✨

    Latest suggestions up to f6bf753

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Declare isCommunity prop

    The identifier isCommunity is not declared in this scope, causing a compile error.
    Destructure or define isCommunity from props (or compute it) before use.

    packages/client/src/components/designer/AddBlocksMenuCard.tsx [195-205]

    +const { isCommunity } = props;
     state.dispatch({
         message: ActionMessages.ADD_BLOCK,
         payload: {
             json: item.json,
             position: {
                 parent: siblingWidget.parent.id,
                 slot: siblingWidget.parent.slot,
                 sibling: siblingWidget.id,
                 type: placeholderAction.type,
             },
    -        isCommunity: isCommunity,
    +        isCommunity,
         },
     }) as string;
    Suggestion importance[1-10]: 7

    __

    Why: The isCommunity identifier is undefined in this component scope and must be destructured from props to avoid a compile error.

    Medium
    Use collision-resistant IDs

    Using Math.random() with a small range risks ID collisions. Switch to a more robust
    unique ID generator such as a timestamp or UUID for collision resistance.

    libs/renderer/src/store/state/state.store.ts [2051]

    -const newQueryId = `com_${key}_${Math.floor(Math.random() * 1000)}`;
    +const newQueryId = `com_${key}_${Date.now()}`;
    Suggestion importance[1-10]: 4

    __

    Why: Switching from Math.random() to a timestamp or UUID reduces ID collision risk, though there are still better options like proper UUIDs.

    Low
    General
    Default isCommunity to false

    The optional isCommunity parameter can be undefined and may lead to ambiguous
    truthiness checks. Provide a default value of false for clarity and to avoid passing
    undefined.

    libs/renderer/src/store/state/state.store.ts [1246-1250]

     private addBlock = (
         json: BlockJSON,
         position?: AddBlockAction["payload"]["position"],
    -    isCommunity?: boolean,
    +    isCommunity: boolean = false,
     ): string => {
    Suggestion importance[1-10]: 4

    __

    Why: Providing a default false value for the optional isCommunity parameter avoids ambiguous undefined checks and clarifies intent.

    Low

    Previous suggestions

    Suggestions up to commit 6175425
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Preserve slot children key

    The processSlots function strips out the children arrays by only keeping keys in
    allowedKeys, preventing recursive traversal of nested blocks. Add 'children' to
    allowedKeys so that slot children are preserved and findUpdatedBlockId can correctly
    locate nested blocks.

    packages/client/src/components/designer/AddBlocksMenuCard.tsx [297-303]

     const allowedKeys = [
         'widget',
         'data',
         'listeners',
         'slots',
         'id',
         'referenceId',
    +    'children',
     ];
    Suggestion importance[1-10]: 8

    __

    Why: Without 'children' in allowedKeys, the processSlots function drops the children arrays and breaks nested block traversal, so adding it prevents missing nested blocks.

    Medium
    Exclude block id from query scan

    The scan is mistakenly treating every node.id (including block IDs) as a query ID.
    Limit detection to payload.queryId and queryId only, removing the fallback to
    node.id so block identifiers aren’t misclassified as queries.

    packages/client/src/components/designer/AddClientBlockModal.tsx [143-146]

     const maybeId =
         (node as any).payload?.queryId ??
    -    (node as any).queryId ??
    -    (node as any).id; // <- adjust if your schema differs
    +    (node as any).queryId; // only query identifiers
    Suggestion importance[1-10]: 8

    __

    Why: Treating every node.id as a query ID will misclassify block IDs as queries and corrupt the scan logic, so limiting to payload.queryId and queryId fixes this bug.

    Medium
    General
    Avoid global regex side-effects

    Using a global regex with .test() can lead to stateful matches and skipped
    replacements. Remove the 'g' flag or recreate the regex for each test to avoid side
    effects on lastIndex.

    packages/client/src/components/designer/AddBlocksMenuCard.tsx [226-229]

     const variableNameRegex = new RegExp(
    -    `{{\\s*${targetVariableName}(\\.[^}]+)?\\s*}}`,
    -    'g',
    -);
    +    `{{\\s*${targetVariableName}(\\.[^}]+)?\\s*}}`
    +); // no 'g' flag
    Suggestion importance[1-10]: 5

    __

    Why: A global regex used with .test() introduces stateful lastIndex behavior that can skip matches, so removing the 'g' flag ensures reliable detection.

    Low
    Use timestamp for unique IDs

    Generating new query IDs with Math.random() risks collisions and makes debugging
    harder. Use a timestamp or a deterministic unique suffix (e.g. Date.now()) to ensure
    IDs are unique.

    packages/client/src/components/designer/AddBlocksMenuCard.tsx [187]

    -const newQueryId = `${key}_${Math.floor(Math.random() * 1000)}`;
    +const newQueryId = `${key}_${Date.now()}`;
    Suggestion importance[1-10]: 4

    __

    Why: Switching from Math.random() to Date.now() reduces collision risk and aids debugging, but it's a style/robustness enhancement rather than a critical fix.

    Low

    @Paulson-Robert Paulson-Robert marked this pull request as draft July 9, 2025 13:39
    @Paulson-Robert Paulson-Robert marked this pull request as ready for review July 11, 2025 11:05
    @johbaxter
    Copy link
    Copy Markdown
    Contributor

    @KirthikaKumar-K can we merge your community block design changes into this branch

    @KirthikaKumar-K
    Copy link
    Copy Markdown
    Contributor

    @KirthikaKumar-K can we merge your community block design changes into this branch

    sure @johbaxter

    @Paulson-Robert
    Copy link
    Copy Markdown
    Contributor Author

    @KirthikaKumar-K can we merge your community block design changes into this branch

    But @johbaxter her ticket has only FE changes with incomplete features associated with incomplete BE pieces. If we merge it, 1340 ticket can be closed only after the whole implementation. Is it ok?

    @johbaxter
    Copy link
    Copy Markdown
    Contributor

    Okay yeah lets hold off merging then @KirthikaKumar-K. We can handle your changes independently.

    Do you mind linking your branch in a comment in this thread @KirthikaKumar-K

    @KirthikaKumar-K
    Copy link
    Copy Markdown
    Contributor

    Okay yeah lets hold off merging then @KirthikaKumar-K. We can handle your changes independently.

    Do you mind linking your branch in a comment in this thread @KirthikaKumar-K

    Sure @johbaxter ,
    Here is the PR.
    This is the Branch.

    @johbaxter
    Copy link
    Copy Markdown
    Contributor

    johbaxter commented Jul 30, 2025

    1. Create app and drop the json saved to this comment.

    CONTAINER_TO_COMMUNITY.json

    image

    I want to make this container-1 a community block

    Looks like we save the data structure to the db correctly, even though i have some thoughts on a refactor of how we save.

    But cool now its saved in the community blocks.

    1. Create a new app and drag in community block
    image

    You'll notice some of the run cell actions don't persist when dragging in the community block. As well as no variable/id in the 2nd notebook cell that got dropped in. Is it due to how we saved it?

    Outside of this it does seem the variables do port over.

    We will need to take another look at how we do id reassign later on, but this bug fix will need to be done before then (prototype still)


    Along with this i will need you to test dragging in the community block, when there is already established blocks and notebooks in an app

    1. Make a template app for "Create Diabetes Record"
    2. Drag above community block into this (do the ids/variables assign correctly)
    3. Make a template app for "Ask LLM"
    4. Do step 2 again.

    @Paulson-Robert
    Copy link
    Copy Markdown
    Contributor Author

    @johbaxter
    Found the cause and added a fix for the above explained issue - commit

    • Cause : In the block JSON, as listener which is of RUN_CELL type does not have the info about its alias name. It is referenced as cell id(eg: 1, 2).
    • Fix : Now I have used the getAlias method to find the alias name and include the associated variable to it.

    Yes, the issue is the way we save the json, it has missed some variables.

    I have tested using community along with Template apps with existing vars. Variables and Queries are getting assigned properly.
    You can give it a check now.

    @johbaxter
    Copy link
    Copy Markdown
    Contributor

    We will address a refactor with migration functions at a later point. Will push this for the purpose of demo.

    I have thoughts on the

    1. structure of how we save to db (will blast db in v2)
    2. communityBlockMapping saved on blocks
    3. id generation prefixed with comm_

    data: {},
    listeners: {},
    slots: {},
    communityBlockMapping: {},
    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    I don't want this added

    type: "after";
    sibling: string;
    };
    isCommunity?: boolean; // Optional flag to indicate if the block is community-based
    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Will look at later point but is this needed.

    const { json, position, isCommunity } = action.payload;

    return this.addBlock(json, position);
    return this.addBlock(json, position, isCommunity);
    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Wondering if we need isCommunity will explore in a refactor

    Copy link
    Copy Markdown
    Contributor Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Need some flag to differentiate in triggering addBlock dispatch for normal block vs community block

    @johbaxter johbaxter merged commit 03081b9 into dev Jul 31, 2025
    3 checks passed
    @johbaxter johbaxter deleted the 1340-Create-Notebook-Community-Block-Tie branch July 31, 2025 15:55
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-07-31 *

    Added

    • Introduce support for community blocks with an isCommunity flag and source-to-local ID mapping
    • Auto-scan and inject dependent notebook queries and variables when dropping community blocks
    • Update block and variable ID generation to include community-specific prefixes and mappings

    to commit the new content to the CHANGELOG.md file, please type:
    '/update_changelog --pr_update_changelog.push_changelog_changes=true'

    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.

    Create Notebook / Community Block Tie

    4 participants