Skip to content

feat: history for flow inputs #5117

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

Merged
merged 91 commits into from
Jan 30, 2025
Merged

feat: history for flow inputs #5117

merged 91 commits into from
Jan 30, 2025

Conversation

Guilhem-lm
Copy link
Contributor

@Guilhem-lm Guilhem-lm commented Jan 22, 2025

  • Use new history and saved input picker in detail page

Important

Adds history and saved input management for flows with JSON view and input selection features.

  • Behavior:
    • Introduces history and saved input management for flows in FlowPreviewContent.svelte and FlowStatusViewerInner.svelte.
    • Adds JSON view toggle for input arguments in RunForm.svelte and FlowInput.svelte.
    • Implements input selection badges in InputSelectedBadge.svelte.
  • Components:
    • Adds JsonInputs.svelte for JSON input handling.
    • Adds SavedInputsV2.svelte and SchemaFormWithArgPicker.svelte for managing saved and historical inputs.
    • Modifies FlowPreviewContent.svelte to use SchemaFormWithArgPicker.
  • Misc:
    • Refactors alert styles into model.ts for reuse.
    • Updates Drawer.svelte and Disposable.svelte to handle escape key behavior with preventEscape flag.
    • Adjusts Popover.svelte to support hover-based opening.

This description was created by Ellipsis for 42ae9ff. It will automatically update as commits are pushed.

@Guilhem-lm Guilhem-lm marked this pull request as ready for review January 29, 2025 16:16
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 3d8e7ed in 1 minute and 28 seconds

More details
  • Looked at 2122 lines of code in 28 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/lib/components/schema/SchemaPickerRow.svelte:85
  • Draft comment:
    Ensure consistent handling of 'WINDMILL_TOO_BIG' payloads across all components that process payloads.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code in SchemaPickerRow.svelte has a potential issue with handling large payloads. The condition for 'WINDMILL_TOO_BIG' is correctly handled, but it's important to ensure that this condition is consistently checked wherever payloads are processed.
2. frontend/src/lib/components/meltComponents/Popover.svelte:32
  • Draft comment:
    Consider debouncing the mouseenter and mouseleave events to prevent rapid state changes.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code in Popover.svelte has a potential issue with the openOnHover functionality. The mouseenter and mouseleave events directly modify the open state, which might lead to unexpected behavior if the state is also controlled externally.

Workflow ID: wflow_HKN3UDGaFSc8nCbp


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 7d0af65 in 44 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/lib/components/SchemaFormWithArgPicker.svelte:102
  • Draft comment:
    Typo in event type. Change 'histroy' to 'history'.
									dispatch('select', { payload: e.detail, type: 'history' })
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_3ksjMUwzYZb1NuaJ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on ff6e3f6 in 52 seconds

More details
  • Looked at 150 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. frontend/src/lib/components/triggers/CaptureTable.svelte:83
  • Draft comment:
    Avoid using console.log for debugging in production code. Consider using a proper logging mechanism or remove it if not needed.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    While console.log statements generally shouldn't be in production code, this one appears to be intentionally logging a specific condition (WINDMILL_TOO_BIG) that might be useful for debugging in production. However, a proper logging mechanism would be better. The comment is technically correct but may be too minor of an issue to warrant a PR comment.
    The console.log might actually be intentional for monitoring this edge case in production. Also, this is a relatively minor code quality issue that doesn't affect functionality.
    While the logging could be intentional, using console.log is still not best practice and a proper logging system would be better. However, this feels too minor for a PR comment.
    While technically correct, this comment is too minor and doesn't warrant blocking or modifying the PR. The console.log, while not ideal, isn't causing any real issues.
2. frontend/src/lib/components/triggers/CaptureTable.svelte:146
  • Draft comment:
    Consider checking the type of capture.payloadData before using structuredClone to avoid unnecessary cloning if it's already a simple object.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The getPayload function uses structuredClone on capture.payloadData, which might be unnecessary if capture.payloadData is already a simple object. This could be optimized by checking the type before cloning.
3. frontend/src/lib/components/triggers/CaptureTable.svelte:130
  • Draft comment:
    Consider checking the type of payloadData before using structuredClone to avoid unnecessary cloning if it's already a simple object.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The handleSelect function uses structuredClone on payloadData, which might be unnecessary if payloadData is already a simple object. This could be optimized by checking the type before cloning.

Workflow ID: wflow_6tcgzhHmFyYPUab7


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 8a0a174 in 1 minute and 12 seconds

More details
  • Looked at 53 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_QvoddDTRXCSBYYQZ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 3ac03ee in 28 seconds

More details
  • Looked at 174 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_4LBQYHqmEK6MBxXR


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@rubenfiszel rubenfiszel changed the title glm/captures_v4 feat: history for flow inputs Jan 29, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 7ce6368 in 22 seconds

More details
  • Looked at 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/lib/components/triggers/CaptureTable.svelte:82
  • Draft comment:
    Removing the console.log statement is a good practice for production code to avoid unnecessary logging.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The removal of the console.log statement is appropriate for production code as it avoids unnecessary logging, which can clutter the console and potentially expose sensitive information.

Workflow ID: wflow_3ReH0smxkMuHJMuF


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Incremental review on 7a931db in 1 minute and 35 seconds

More details
  • Looked at 222 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/lib/components/meltComponents/SideBarTab.svelte:38
  • Draft comment:
    The class binding syntax is incorrect. Use the correct Svelte syntax for conditional classes.
	class="flex flex-col relative !overflow-visible {expandRight ? 'pr-2 -mr-2' : ''}"
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. frontend/src/lib/components/SchemaFormWithArgPicker.svelte:132
  • Draft comment:
    Typo: Correct svelete:fragment to svelte:fragment. This typo appears multiple times in the code.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_7OeGyOj0VFvO65uD


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

<div style="height: {rightHeight}px" class="border-t border-r pb-2">
{#if selectedTab === 'history'}
<FlowInputEditor title="History">
<svelete:fragment slot="action">
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: Correct svelete:fragment to svelte:fragment. This typo appears multiple times in the code.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 42ae9ff in 24 seconds

More details
  • Looked at 15 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/lib/components/triggers/CaptureTable.svelte:331
  • Draft comment:
    Consider simplifying the nested ternary operation for the title attribute to improve readability.
 title={isFlow && testKind === 'main' ? 'Test flow with args' : (testKind === 'preprocessor' ? 'Apply args to preprocessor' : 'Apply args to inputs')}
  • Reason this comment was not posted:
    Confidence changes required: 20%
    The code uses a ternary operator for setting the title attribute of a button. The logic is correct, but the readability can be improved by using a single ternary operation instead of nested ones.

Workflow ID: wflow_3znNhyOeufiPE6Ze


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@rubenfiszel rubenfiszel merged commit cd44056 into main Jan 30, 2025
2 of 3 checks passed
@rubenfiszel rubenfiszel deleted the glm/captures_v4 branch January 30, 2025 15:30
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants