Skip to content

Conversation

@Eric-B-Wu
Copy link
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Removing ParseWithMetadata Experiment Flags

Impact of Change

  • Users: No longer needing an experiment flag for parseWithMetadata
  • Developers: No no longer a check for enableParseDocumentWithMetadata
  • System:

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Contributors

@Eric-B-Wu

Screenshots/Videos

Copilot AI review requested due to automatic review settings December 4, 2025 21:02
@Eric-B-Wu Eric-B-Wu added the risk:low Low risk change with minimal impact label Dec 4, 2025
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: chore(designer): Remove ParseWithMetadata filter exp flag
  • Issue: Title is clear, concise, and descriptive, using conventional commit style.
  • Recommendation: No changes needed.

Commit Type

  • Properly selected (chore)
  • Only one selected, which is correct.

Risk Level

  • Clearly marked as "Low" in both body and label, and matches the actual scope (removing experiment flags, not direct user-impacting functionality).

⚠️ What & Why

  • Current: Removing ParseWithMetadata Experiment Flags
  • Issue: The explanation is quite brief and could include a little more about why this flag is no longer needed or any context about its history.
  • Recommendation: Consider expanding with a bit more context. Example: The ParseWithMetadata experiment has been validated and is now considered stable, so the related experiment flags are redundant and can be safely removed.

⚠️ Impact of Change

  • Users and Developers sections are filled, but 'System' impact is left blank.
  • Recommendation:
    • Users: No longer needing an experiment flag for parseWithMetadata
    • Developers: No longer a check for enableParseDocumentWithMetadata
    • System: Please clarify if there is no system impact, or use None/N/A for clarity.

Test Plan

  • Manual testing checked, and other boxes intentionally left unchecked. This is acceptable for this change scope.

⚠️ Contributors

  • Only self (@Eric-B-Wu) tagged. No issue, but as a best practice, remember to give credit to others (PMs/designers) if applicable.

Screenshots/Videos

  • Not needed for this type of change. Correctly left blank.

Summary Table

Section Status Recommendation
Title
Commit Type
Risk Level
What & Why ⚠️ Slightly expand context in the explanation
Impact of Change ⚠️ Add System: N/A or clarify if no impact
Test Plan
Contributors ⚠️ Tag others if they contributed
Screenshots/Videos

The PR meets all required criteria and passes! Please make minor improvements to the What & Why and Impact of Change sections for clarity, if convenient, but this is not blocking. Thank you for maintaining quality standards!


Last updated: Thu, 04 Dec 2025 21:02:49 GMT

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: chore(designer): Remove ParseWithMetadata filter exp flag
  • Issue: None
  • Recommendation: The title is clear and follows conventional commit format. No changes needed.

Commit Type

  • Properly selected (chore - Maintenance/tooling).
  • Only one checked, correct usage.

Risk Level

  • Selected as Low, matches the label Risk:Low and seems consistent with the scope (removal of an experiment flag).

⚠️ What & Why

  • Current: Removing ParseWithMetadata Experiment Flags
  • Issue: Brief but could use slightly more context about why it's safe to remove or what necessitated this.
  • Recommendation: Optionally provide extra detail for future reference (e.g. "Flag has been live/default for a release; no longer needed and causes tech debt.").

⚠️ Impact of Change

  • Users: Clear impact noted (no longer need experiment flag).
  • Developers: Clear (removal of check).
  • System: Section left blank.
  • Recommendation: For completeness, state: "No system/architecture impact expected."

Test Plan

  • Manual testing was performed; no unit or E2E checks added/updated (reasonable for a guard flag removal that doesn't change core logic).

⚠️ Contributors

  • Only self-tagged. It passes, but as a nudge: Tag PMs, designers, or others if they contributed, or add a comment: "No external contributors."

Screenshots/Videos

  • Not required for this type of change.

Summary Table

Section Status Recommendation
Title
Commit Type
Risk Level
What & Why ⚠️ Optionally add a sentence about the history/impact of this flag removal
Impact of Change ⚠️ Fill out 'System' field explicitly as 'No architectural impact'
Test Plan
Contributors ⚠️ Tag or comment on contributor section if missing
Screenshots/Videos

Your PR passes. For very high standards, consider adding a brief note on "Why now?" in the 'What & Why' and explicitly mark 'System' in Impact. Thank you for maintaining great hygiene in your PR templates!


Last updated: Thu, 04 Dec 2025 21:02:49 GMT

@Eric-B-Wu Eric-B-Wu enabled auto-merge (squash) December 4, 2025 21:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the ParseWithMetadata experiment flag, making the parsedocumentwithmetadata operation permanently available in the designer UI. The change eliminates conditional feature toggling that was previously controlled via experimentation flags.

Key changes:

  • Removed the experiment flag definition and enablement function from the shared services
  • Eliminated the hook that checked the flag state in both designer and designer-v2 libraries
  • Simplified operation search and filtering logic to no longer conditionally hide the operation

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
libs/logic-apps-shared/src/designer-client-services/lib/experimentationFlags.ts Removed ENABLE_PARSE_DOCUMENT_WITH_METADATA flag constant and enableParseDocumentWithMetadata() function
libs/designer/src/lib/ui/panel/recommendation/searchView.tsx Removed hook usage and flag parameter from DefaultSearchOperationsService constructor
libs/designer/src/lib/ui/panel/recommendation/operationGroupDetailView.tsx Removed hook usage and conditional filtering logic based on the flag
libs/designer/src/lib/ui/panel/recommendation/hooks.ts Removed useShouldEnableParseDocumentWithMetadata hook implementation
libs/designer/src/lib/ui/panel/recommendation/SearchOpeationsService.tsx Removed showParseDocWithMetadata constructor parameter and filtering logic
libs/designer-v2/src/lib/ui/panel/recommendation/searchView.tsx Removed hook usage and flag parameter from DefaultSearchOperationsService constructor
libs/designer-v2/src/lib/ui/panel/recommendation/hooks.ts Removed useShouldEnableParseDocumentWithMetadata hook implementation
libs/designer-v2/src/lib/ui/panel/recommendation/details/connectorDetailsView.tsx Removed hook usage and conditional filtering logic based on the flag
libs/designer-v2/src/lib/ui/panel/recommendation/SearchOpeationsService.tsx Removed showParseDocWithMetadata constructor parameter and filtering logic

@Eric-B-Wu Eric-B-Wu merged commit 8fcb619 into main Dec 4, 2025
21 checks passed
@Eric-B-Wu Eric-B-Wu deleted the eric/removeParseWithMetadata branch December 4, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants