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

refactor(agents-api): Minor refactors to typespec types #465

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented Aug 20, 2024

Signed-off-by: Diwank Tomer [email protected]


🚀 This description was created by Ellipsis for commit 8ab6dea

Summary:

Refactored typespec types in agents-api for improved type safety and consistency across Python and TypeScript models and schemas.

Key points:

  • Refactored typespec types in agents-api for improved type safety and consistency.
  • Removed extra="allow" from ConfigDict in agents-api/agents_api/autogen/* models.
  • Added max_length constraints to string fields in models like Agent, CreateAgentRequest, JobStatus.
  • Updated type annotations for fields in models for clarity and consistency.
  • Refactored TypeScript models and schemas in sdks/ts/src/api/models/* and sdks/ts/src/api/schemas/* to align with updated typespec definitions.
  • Ensured TypeScript code generation reflects updated typespec types, including changes to ChatMLMessage, Tasks_PromptStep, etc.
  • Modified typespec/common/scalars.tsp to include maxLength and pattern constraints for scalars like identifierSafeUnicode and validPythonIdentifier.

Generated with ❤️ by ellipsis.dev

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! Reviewed everything up to 744e2ef in 1 minute and 16 seconds

More details
  • Looked at 2792 lines of code in 58 files
  • Skipped 2 files when reviewing.
  • Skipped posting 8 drafted comments based on config settings.
1. agents-api/agents_api/autogen/Agents.py:13
  • Draft comment:
    Removing extra="allow" from ConfigDict might cause issues if there are unexpected fields in the input data. Ensure that this change is intentional and that the system does not rely on this flexibility. This applies to other similar changes in the file.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is speculative and asks the author to ensure the change is intentional, which is against the rules. The removal of extra="allow" is a code change, but the comment does not provide a definite issue or actionable feedback. It is more of a cautionary note rather than a necessary code change suggestion.
    I might be overlooking the potential impact of removing extra="allow", which could be significant if the system relies on it. However, the comment does not provide a definite issue or solution, making it speculative.
    While the removal of extra="allow" could have implications, the comment does not provide a clear, actionable issue. It remains speculative and does not align with the rules for comments.
    The comment should be removed as it is speculative and does not provide a definite issue or actionable feedback.
2. agents-api/agents_api/autogen/Agents.py:29
  • Draft comment:
    Ensure that the max_length and pattern constraints align with the expected data for this field. This applies to other similar changes in the file.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The max_length and pattern constraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
3. agents-api/agents_api/autogen/Chat.py:250
  • Draft comment:
    Ensure that the max_length and pattern constraints align with the expected data for this field. This applies to other similar changes in the file.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The max_length and pattern constraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
4. agents-api/agents_api/autogen/Docs.py:32
  • Draft comment:
    Ensure that the max_length constraint aligns with the expected data for this field. This applies to other similar changes in the file.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The max_length and pattern constraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
5. agents-api/agents_api/autogen/Executions.py:110
  • Draft comment:
    Ensure that the max_length and pattern constraints align with the expected data for this field. This applies to other similar changes in the file.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The max_length and pattern constraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
6. agents-api/agents_api/autogen/Tools.py:43
  • Draft comment:
    Ensure that the max_length and pattern constraints align with the expected data for this field. This applies to other similar changes in the file.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The max_length and pattern constraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
7. agents-api/agents_api/autogen/Users.py:25
  • Draft comment:
    Ensure that the max_length and pattern constraints align with the expected data for this field. This applies to other similar changes in the file.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The max_length and pattern constraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.
8. typespec/common/scalars.tsp:15
  • Draft comment:
    Ensure that the maxLength and pattern constraints align with the expected data for this scalar. This applies to other similar changes in the file.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The max_length and pattern constraints added to fields are good for validation, but they should be consistent with the intended use of these fields. Ensure that these constraints align with the expected data.

Workflow ID: wflow_R4A59dapLtK85TgP


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 8ab6dea in 1 minute and 37 seconds

More details
  • Looked at 4483 lines of code in 89 files
  • Skipped 2 files when reviewing.
  • Skipped posting 7 drafted comments based on config settings.
1. sdks/ts/src/api/models/Tasks_ErrorWorkflowStep.ts:9
  • Draft comment:
    The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.
  • Reason this comment was not posted:
    Marked as duplicate.
2. sdks/ts/src/api/models/Tasks_EvaluateStep.ts:10
  • Draft comment:
    The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.
  • Reason this comment was not posted:
    Marked as duplicate.
3. sdks/ts/src/api/models/Tasks_ForeachStep.ts:10
  • Draft comment:
    The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.
  • Reason this comment was not posted:
    Marked as duplicate.
4. sdks/ts/src/api/models/Tasks_GetStep.ts:9
  • Draft comment:
    The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.
  • Reason this comment was not posted:
    Marked as duplicate.
5. sdks/ts/src/api/models/Tasks_IfElseWorkflowStep.ts:23
  • Draft comment:
    The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.
  • Reason this comment was not posted:
    Marked as duplicate.
6. sdks/ts/src/api/models/Tasks_LogStep.ts:10
  • Draft comment:
    The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.
  • Reason this comment was not posted:
    Marked as duplicate.
7. sdks/ts/src/api/models/Tasks_ParallelStep.ts:17
  • Draft comment:
    The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_fjLwHc1BgoVtIjoV


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.

/**
* The kind of step
*/
readonly kind_: "embed";
Copy link
Contributor

Choose a reason for hiding this comment

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

The kind_ property is defined twice. Consider removing the redundant definition to avoid confusion and maintain consistency.

@whiterabbit1983 whiterabbit1983 merged commit 1c071ec into dev-tasks-disable-routes Aug 21, 2024
2 of 5 checks passed
@whiterabbit1983 whiterabbit1983 deleted the r/typespec-refactors branch August 21, 2024 13:50
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.

2 participants