Skip to content
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

[Number Series Copilot] Bug Fixes in Intent Understanding and Field Validation #2111

Merged

Conversation

DmitryKatson
Copy link
Contributor

@DmitryKatson DmitryKatson commented Sep 27, 2024

Summary

This PR addresses three critical bugs in the Number Series Copilot:

  1. Bug 1: Incorrect Intent Mapping for "Generate" Command

    • The system was misinterpreting the "generate" command, often preparing Number Series for the next period instead of creating a new one. The fix involves updating the tools definition and prompt to clarify the distinction between creating new Number Series and generating Number Series for the next period.
  2. Bug 2: Incorrect Field Check for Existing Number Series

    • The system was incorrectly performing checks on all fields for existing Number Series. The fix modifies the behavior to exclude pre-existing Number Series from the field validation checks, ensuring that only mandatory fields are validated for AI generated series.
  3. Bug 3: Blank Fields Causing Validation Errors

    • The system was generating Number Series without values for optional fields, causing validation errors. The fix refines the validation logic so that only mandatory fields are checked, allowing optional fields to remain blank if necessary.

Steps to Implement:

  1. Refactor the LLM prompt and tools definition to improve the understanding of user intent between creating and preparing Number Series for different periods.
  2. Update the logic to exclude field checks for existing Number Series and restrict checks to mandatory fields.

Acceptance Criteria:

  • Users can create Number Series without mistakenly triggering next-period Number Series generation when using the word "generate."
  • Existing Number Series are excluded from unnecessary checks, and only mandatory fields are validated for new series.

These fixes aim to enhance the accuracy and reliability of the Number Series Copilot, ensuring it correctly interprets user commands and validates the necessary fields.

Work Item(s)

Fixes #2106

Fixes AB#550892, AB#544073

…ation

Summary: This commit resolves three bugs in the Number Series generation process. It improves intent mapping for the "Generate" command, corrects field checks for existing Number Series, and refines validation logic to allow optional fields to be blank. These fixes enhance the accuracy and reliability of Number Series generation.

Fixes microsoft#2106
@DmitryKatson DmitryKatson requested review from a team as code owners September 27, 2024 15:34
@github-actions github-actions bot added AL: Business Foundation From Fork Pull request is coming from a fork labels Sep 27, 2024
@DmitryKatson
Copy link
Contributor Author

DmitryKatson commented Sep 27, 2024

@JesperSchulz you should update:

  • ToolsSelectionPrompt.txt
  • Tool3Definition.txt

BUT Important! These should have new version starting from v25.1 . For the v25.0 they should stay current, as they aligned with the function in AL that changed name in v25.1

@JesperSchulz JesperSchulz self-assigned this Sep 30, 2024
@JesperSchulz JesperSchulz added the Linked Issue is linked to a Azure Boards work item label Sep 30, 2024
@github-actions github-actions bot added this to the Version 26.0 milestone Sep 30, 2024
Copy link
Contributor

@JesperSchulz JesperSchulz left a comment

Choose a reason for hiding this comment

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

Other than the pending TODOs, I'm fine with this PR. But let's wait for Darricks return to see if we cannot find a more elegant solution of versioning prompts, other than creating new ones which replace the old ones.

@JesperSchulz JesperSchulz added the Integration GitHub request for Integration area label Sep 30, 2024
@JesperSchulz
Copy link
Contributor

@DmitryKatson, we should get this ready. Could you ensure the build passes?

@DmitryKatson
Copy link
Contributor Author

@JesperSchulz @darjoo fixed previous build errors

@DmitryKatson
Copy link
Contributor Author

@JesperSchulz JesperSchulz enabled auto-merge (squash) October 23, 2024 09:27
@JesperSchulz JesperSchulz merged commit 276deb8 into microsoft:main Oct 23, 2024
16 checks passed
JesperSchulz pushed a commit that referenced this pull request Oct 23, 2024
…alidation (#2111)

<!-- Thank you for submitting a Pull Request. If you're new to
contributing to BCApps please read our pull request guideline below
* https://github.com/microsoft/BCApps/Contributing.md
-->

#### Summary <!-- Provide a general summary of your changes -->

This PR addresses three critical bugs in the Number Series Copilot:

1. **Bug 1: Incorrect Intent Mapping for "Generate" Command**
- The system was misinterpreting the "generate" command, often preparing
Number Series for the next period instead of creating a new one. The fix
involves updating the tools definition and prompt to clarify the
distinction between creating new Number Series and generating Number
Series for the next period.

2. **Bug 2: Incorrect Field Check for Existing Number Series**
- The system was incorrectly performing checks on all fields for
existing Number Series. The fix modifies the behavior to exclude
pre-existing Number Series from the field validation checks, ensuring
that only mandatory fields are validated for AI generated series.

3. **Bug 3: Blank Fields Causing Validation Errors**
- The system was generating Number Series without values for optional
fields, causing validation errors. The fix refines the validation logic
so that only mandatory fields are checked, allowing optional fields to
remain blank if necessary.

**Steps to Implement:**

1. Refactor the LLM prompt and tools definition to improve the
understanding of user intent between creating and preparing Number
Series for different periods.
2. Update the logic to exclude field checks for existing Number Series
and restrict checks to mandatory fields.

**Acceptance Criteria:**

- Users can create Number Series without mistakenly triggering
next-period Number Series generation when using the word "generate."
- Existing Number Series are excluded from unnecessary checks, and only
mandatory fields are validated for new series.

These fixes aim to enhance the accuracy and reliability of the Number
Series Copilot, ensuring it correctly interprets user commands and
validates the necessary fields.


#### Work Item(s) <!-- Add the issue number here after the #. The issue
needs to be open and approved. Submitting PRs with no linked issues or
unapproved issues is highly discouraged. -->

Fixes #2106 

Fixes
[AB#550892](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/550892),
[AB#544073](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/544073)
JesperSchulz added a commit that referenced this pull request Oct 23, 2024
…ield V… (#2251)

…alidation (#2111)

<!-- Thank you for submitting a Pull Request. If you're new to
contributing to BCApps please read our pull request guideline below
* https://github.com/microsoft/BCApps/Contributing.md -->

#### Summary <!-- Provide a general summary of your changes -->

This PR addresses three critical bugs in the Number Series Copilot:

1. **Bug 1: Incorrect Intent Mapping for "Generate" Command**
- The system was misinterpreting the "generate" command, often preparing
Number Series for the next period instead of creating a new one. The fix
involves updating the tools definition and prompt to clarify the
distinction between creating new Number Series and generating Number
Series for the next period.

2. **Bug 2: Incorrect Field Check for Existing Number Series**
- The system was incorrectly performing checks on all fields for
existing Number Series. The fix modifies the behavior to exclude
pre-existing Number Series from the field validation checks, ensuring
that only mandatory fields are validated for AI generated series.

3. **Bug 3: Blank Fields Causing Validation Errors**
- The system was generating Number Series without values for optional
fields, causing validation errors. The fix refines the validation logic
so that only mandatory fields are checked, allowing optional fields to
remain blank if necessary.

**Steps to Implement:**

1. Refactor the LLM prompt and tools definition to improve the
understanding of user intent between creating and preparing Number
Series for different periods.
2. Update the logic to exclude field checks for existing Number Series
and restrict checks to mandatory fields.

**Acceptance Criteria:**

- Users can create Number Series without mistakenly triggering
next-period Number Series generation when using the word "generate."
- Existing Number Series are excluded from unnecessary checks, and only
mandatory fields are validated for new series.

These fixes aim to enhance the accuracy and reliability of the Number
Series Copilot, ensuring it correctly interprets user commands and
validates the necessary fields.


#### Work Item(s) <!-- Add the issue number here after the #. The issue
needs to be open and approved. Submitting PRs with no linked issues or
unapproved issues is highly discouraged. -->

Fixes #2106 

Fixes
[AB#555649](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/555649)

Co-authored-by: Dmitry Katson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AL: Business Foundation From Fork Pull request is coming from a fork Integration GitHub request for Integration area Linked Issue is linked to a Azure Boards work item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: [Number Series Copilot] Bug fixes
3 participants