-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(copilot): null check simplified #1431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR replaces the recursive pruneNullishDeep function with a simpler removeNullish function in the copilot blocks metadata tool. The change simplifies null checking by only removing null/undefined values at the top level of objects, rather than recursively cleaning nested structures.
Key Changes:
- Removed complex recursive
pruneNullishDeepfunction (23 lines) - Added simpler
removeNullishfunction (12 lines) that only handles top-level filtering - Updated function call on line 233 to use the new function name
Concerns:
- The new implementation may leave null/undefined values in nested objects and arrays
- This could potentially cause issues downstream if consumer code expects fully cleaned metadata
- The change reduces the thoroughness of null value filtering without clear justification
Confidence Score: 2/5
- This PR introduces potential regressions by removing deep null cleaning functionality
- Score reflects the significant change in behavior from recursive to shallow null removal, which could leave problematic null values in nested metadata structures and potentially break downstream consumers
- The get-blocks-metadata-tool.ts file requires attention due to the behavioral change in null handling
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/lib/copilot/tools/server/blocks/get-blocks-metadata-tool.ts | 2/5 | Replaced recursive pruneNullishDeep function with shallow removeNullish function, potentially leaving null values in nested objects |
Sequence Diagram
sequenceDiagram
participant Client as Client/Copilot
participant Tool as getBlocksMetadataServerTool
participant Registry as blockRegistry
participant Processor as removeNullish
Client->>Tool: execute({blockIds})
loop for each blockId
Tool->>Registry: blockRegistry[blockId]
Registry-->>Tool: blockConfig
Tool->>Tool: build metadata object
Tool->>Processor: removeNullish(metadata)
Note over Processor: OLD: pruneNullishDeep (recursive)<br/>NEW: removeNullish (shallow only)
Processor-->>Tool: filtered metadata
Tool->>Tool: result[blockId] = metadata
end
Tool-->>Client: {metadata: result}
1 file reviewed, 2 comments
| function removeNullish(obj: any): any { | ||
| if (!obj || typeof obj !== 'object') return obj | ||
|
|
||
| const cleaned: any = Array.isArray(obj) ? [] : {} | ||
|
|
||
| for (const [key, value] of Object.entries(obj)) { | ||
| if (value !== null && value !== undefined) { | ||
| cleaned[key] = value | ||
| } | ||
| return output as unknown as T | ||
| } | ||
| return value | ||
|
|
||
| return cleaned | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The simplified removeNullish function only removes null/undefined values at the top level, unlike the previous pruneNullishDeep which recursively cleaned nested objects and arrays. This may leave null/undefined values in nested structures that could cause issues downstream.
| function removeNullish(obj: any): any { | ||
| if (!obj || typeof obj !== 'object') return obj | ||
|
|
||
| const cleaned: any = Array.isArray(obj) ? [] : {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The function doesn't handle recursion anymore. If the metadata object contains nested arrays or objects with null/undefined values, they will remain unfiltered.
Summary
Simplify null check for block metadata.
Type of Change
Testing
Locally against staging agent