8000 Updates to native Skill contribution wizard - unify wizards by jeff-phillips-18 · Pull Request #625 · instructlab/ui · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Updates to native Skill contribution wizard - unify wizards #625

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 4, 2025
Merged

Updates to native Skill contribution wizard - unify wizards #625

merged 1 commit into from
Mar 4, 2025

Conversation

jeff-phillips-18
Copy link
Collaborator
@jeff-phillips-18 jeff-phillips-18 commented Feb 27, 2025 •
edited
Loading

Towards SME Submission Wizard Cleanup

Description

  • Bring the Skill contribution wizards up to date with the changes made for the Knowledge wizards:

    • Update the wizard footer
    • Indicate which steps have errors if the user navigates past the step
    • Update step visuals for spacing and fonts

  • Update the form data types to use commonality where possible

  • Remove duplicate wizard pages and pass flags/data to handle the differences between Skill and Knowledge contributions

  • Remove duplicate functionality and create utils to handle the differences

Screen shots

image

image

image

image

image

image

image

/cc @Misjohns

@jeff-phillips-18 jeff-phillips-18 force-pushed the skill-wizard-refactor branch 4 times, most recently from 2ede1f4 to f61f7f9 Compare February 28, 2025 17:19
@vishnoianil
Copy link
Member

@jeff-phillips-18 Thanks for the PR, overall it looks good.

Few minor issues i found during my testing

  • When you "Add Seed Example" it adds the Knowledge seed example (1 context and 2 QnA pair)
  • Due to this, review submission also shows the same knowledge seed example
  • It shows context limit to 500 words but skill form actually has no limit when it comes to context.
  • Although the help text for context says it's option, but it shows error if you don't fill the context.
  • For skills, context is option, so it would probably be better if we show it after the QnA Pair
  • "Upload skill yaml file" feature is failing because it's not able to recognize the json in Skill schema, mostly because we are now using a same data structure for both skill and knowledge.
  • It doesn't display "Yaml content" when i click on Yaml Content from drop down
Cannot read properties of undefined (reading 'split')
    at convertToYaml (SkillWizard.tsx:210:42)
  • It does show attribution content, but the "Download" button is not working.
  • Skill gets submitted successfully, and after that it shows the new skill form, but the Author Information is not auto filled. I think once user submit the skill we should direct user to the dashboard itself.

@jeff-phillips-18
Copy link
Collaborator Author

@jeff-phillips-18 Thanks for the PR, overall it looks good.

Few minor issues i found during my testing

  • When you "Add Seed Example" it adds the Knowledge seed example (1 context and 2 QnA pair)
  • Due to this, review submission also shows the same knowledge seed example

These first two have already been fixed.

  • It shows context limit to 500 words but skill form actually has no limit when it comes to context.
  • Although the help text for context says it's option, but it shows error if you don't fill the context.

This has already been fixed

  • For skills, context is option, so it would probably be better if we show it after the QnA Pair
  • "Upload skill yaml file" feature is failing because it's not able to recognize the json in Skill schema, mostly because we are now using a same data structure for both skill and knowledge.
  • It doesn't display "Yaml content" when i click on Yaml Content from drop down
Cannot read properties of undefined (reading 'split')
    at convertToYaml (SkillWizard.tsx:210:42)
  • It does show attribution content, but the "Download" button is not working.
  • Skill gets submitted successfully, and after that it shows the new skill form, but the Author Information is not auto filled. I think once user submit the skill we should direct user to the dashboard itself.

I'm working on these others

@jeff-phillips-18
Copy link
Collaborator Author
  • Skill gets submitted successfully, and after that it shows the new skill form, but the Author Information is not auto filled. I think once user submit the skill we should direct user to the dashboard itself.

@vishnoianil If we do this the user never sees the notification of the new PR created and doesn't get the link to it. I think we should auto fill the author info again. WDYT?

@vishnoianil
Copy link
Member
  • Skill gets submitted successfully, and after that it shows the new skill form, but the Author Information is not auto filled. I think once user submit the skill we should direct user to the dashboard itself.

@vishnoianil If we do this the user never sees the notification of the new PR created and doesn't get the link to it. I think we should auto fill the author info again. WDYT?

Sounds good.

@vishnoianil
Copy link
Member

@jphillips PR is very close to be perfect now :). All the above listed issues are resolved now.

Found few minor issues with the latest patch:

  • Knowledge form shows 5 QnA pairs now, it should be 3.
  • Skill "Seed Example" step is failing validation. Context is optional but i tested it with context as well, but still validation fails.
  • Download the attribution file (in github mode), shows file name as (skill) and it saves the file with the "yaml" extension. The filename should be attribution.txt. The same happens for knowledge attribution file as well. For knowledge contribution also the filename should be attribution.txt. Although the content of the file looks good.

@jeff-phillips-18 jeff-phillips-18 force-pushed the skill-wizard-refactor branch 3 times, most recently from c6bd0ae to d6f914e Compare March 4, 2025 14:33
@jeff-phillips-18
Create generic contribution wizard, refactor wizards to unify types
7c5bf2a 
Signed-off-by: Jeffrey Phillips <jephilli@redhat.com>
@vishnoianil vishnoianil merged commit 72f70fc into instructlab:main Mar 4, 2025
5 checks passed
@jeff-phillips-18 jeff-phillips-18 deleted the skill-wizard-refactor branch March 4, 2025 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vishnoianil vishnoianil Awaiting requested review from vishnoianil

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jeff-phillips-18 @vishnoianil
0