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

Make review configurable #126

Merged
merged 16 commits into from
Mar 27, 2025
Merged

Make review configurable #126

merged 16 commits into from
Mar 27, 2025

Conversation

LironMShemen
Copy link
Collaborator

No description provided.

Populate the reviewTypes array with `perform`, `analyseScreenAndCreatePilotStep`, `createPrompt` and `extractTaggedOutputs`.
Copy link
Collaborator

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

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

Overall looks very good! 🙌🏼
Round

Copy link
Collaborator

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

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

Great work!!!

Comment on lines 52 to 64
export function extractPilotSummaryOutputs(text: string): {
[K in keyof typeof PILOT_SUMMARY]: string;
} {
return extractTaggedOutputs({ text, outputsMapper: PILOT_SUMMARY });
}

export function extractReviewOutputs(text: string): {
[K in keyof typeof PILOT_REVIEW_SECTION]: string;
} {
return extractTaggedOutputs({ text, outputsMapper: PILOT_REVIEW_SECTION });
}

export function extractAutoPilotStepOutputs(
Copy link
Collaborator

Choose a reason for hiding this comment

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

YES!
This looks WAY BETTER 🙌🏼 🚀

"For each applicable review section (`UX`, `Accessibility`, `Internationalization`), create a comprehensive report enclosed within corresponding tags (e.g., `<UX>`, `<ACCESSIBILITY>`, `<INTERNATIONALIZATION>`), including a summary, findings, and a score out of 10.",
"Ensure each section is clearly labeled and formatted as shown in the examples.",
"If you cannot determine the next action due to ambiguity or missing information, throw an informative error explaining the problem in one sentence.",
"If the goal is achieved, add a `<SUMMARY>` block inside the `<THOUGHTS>` section, summarizing the overall flow and findings.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that how we actually handle the outputs? So this means the thoughts of the last step will always have a <SUMMARY> block inside its contents. Looks like we missed something
I think we should put the summary outside the Thoughts block, and handle it separately..

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here - https://github.com/wix-incubator/pilot/blob/master/packages/core/src/performers/auto-performer/AutoPerformer.ts#L181-L183
we should extract it from the promptResults.

Let's do this change as part of this PR or create a new small task for that, WDYT?

@asafkorem asafkorem requested a review from tzvielwix March 27, 2025 08:53
@asafkorem asafkorem marked this pull request as ready for review March 27, 2025 08:55
@asafkorem asafkorem merged commit 9ebc0d9 into master Mar 27, 2025
3 checks passed
@asafkorem asafkorem deleted the make-review-configurable branch March 27, 2025 12: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.

3 participants