Skip to content

Conversation

jkwatson
Copy link
Collaborator

No description provided.

@jkwatson jkwatson force-pushed the mob/non-stream-tool branch from ae0d1ec to d34b520 Compare June 20, 2025 20:09
@jkwatson jkwatson marked this pull request as ready for review June 22, 2025 15:53
@Copilot Copilot AI review requested due to automatic review settings June 22, 2025 15:53
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Implements fake streaming for non-streaming Bedrock tool-calling models and updates scripts/CI to use prebuilt artifacts.

  • Redirects startup/download scripts to prebuilt_artifacts instead of dynamic downloads.
  • Introduces FakeStreamBedrockConverse wrapper and conditional logic in the tool-calling agent.
  • Adjusts GitHub Actions to install Git LFS and populate the prebuilt_artifacts folder.

Reviewed Changes

Copilot reviewed 9 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
scripts/startup_java.sh Switched jar path from artifacts to prebuilt_artifacts
scripts/refresh_project.sh Disabled downloads, switched extracts to prebuilt_artifacts
scripts/release_version.txt Changed default RELEASE_TAG to dev-testing
.github/workflows/publish_release.yml Added Git LFS steps and copied build outputs into prebuilt_artifacts
.gitattributes Configured LFS for prebuilt_artifacts/*
llm-service/app/services/query/querier.py Added get_model_name, modified model support check for Bedrock models
llm-service/app/services/query/agents/tool_calling_querier.py New fake-stream branching and updated prompt date/time formatting
llm-service/app/services/query/agents/non_streamer_bedrock_converse.py Introduced FakeStreamBedrockConverse class
Comments suppressed due to low confidence (2)

llm-service/app/services/query/agents/non_streamer_bedrock_converse.py:18

  • You've added a new FakeStreamBedrockConverse class but no tests. Adding unit tests to verify that astream_chat_with_tools yields exactly one ChatResponse would help prevent regressions.
class FakeStreamBedrockConverse(BedrockConverse):

llm-service/app/services/query/querier.py:142

  • You compare get_model_name(model_name) against MODIFIED_BEDROCK_FUNCTION_CALLING_MODELS (which is based on raw model names). If get_model_name normalizes differently, this check may never pass. Ensure both use the same normalized key.
    return get_model_name(model_name) in MODIFIED_BEDROCK_FUNCTION_CALLING_MODELS

@jkwatson jkwatson force-pushed the mob/non-stream-tool branch from 2af2941 to f97e1e0 Compare June 22, 2025 15:55
@ewilliams-cloudera ewilliams-cloudera merged commit ba1bc97 into main Jun 24, 2025
3 checks passed
@ewilliams-cloudera ewilliams-cloudera deleted the mob/non-stream-tool branch June 24, 2025 14:52
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.

4 participants