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

PR to update e2e tests for Ansible chatbot service including model evaluation n response. #67

Merged
merged 5 commits into from
Feb 20, 2025

Conversation

justjais
Copy link

@justjais justjais commented Feb 20, 2025

Description

PR to update e2e tests for Ansible chatbot service including model evaluation n response. AAP-39070

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Run the eval test after copying the olsconfig file to parent directory
  • Run the following cmd to test:
OPENAI_API_KEY=IGNORED python -m scripts.evaluation.driver --qna_pool_file ./ansible-chatbot-service/scripts/evaluation/eval_data/aap-sample.parquet --eval_provider_model_id my_rhoai_g3+granite3-8b --eval_metrics answer_relevancy answer_similarity_llm cos_score rougeL_precision --eval_modes ols --judge_model granite3-8b --judge_provider my_rhoai_g3 --eval_query_ids qna1 --eval_api_url https://stage.ai.ansible.redhat.com --eval_api_token_file ../stage_chatbot_token.txt

@@ -96,8 +97,11 @@ def main():
client = Client(base_url=args.eval_api_url, verify=False) # noqa: S501

if "localhost" not in args.eval_api_url:
with open(args.eval_api_token_file, mode="r", encoding="utf-8") as t_f:
token = t_f.read().rstrip()
if path.isfile(args.eval_api_token_file):
Copy link
Author

Choose a reason for hiding this comment

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

this is to accommodate GHA secret retrieval, as we cannot pass secret under a file WRT GHA

Copy link
Collaborator

@TamiTakamiya TamiTakamiya left a comment

Choose a reason for hiding this comment

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

@justjais Generally when we update files that are found in our upstream repo (road-core/service), we open a PR on the upstream repo first, then only deltas that are unique to AAP will be included in this repo.

Changes looked good and I will approve this PR, but would you consider porting some of those changes to road-core/service in the next sprint? Thanks.

@justjais justjais merged commit 610d8f1 into main Feb 20, 2025
22 of 24 checks passed
@justjais justjais deleted the chatbot_eval branch February 20, 2025 13:03
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