FEAT: add prompt suffix handling - #134
Conversation
… Location can be set in the query params
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🔴 | Statements | 54.32% (-0.41% 🔻) |
377/694 |
| 🔴 | Branches | 50.2% (+0.41% 🔼) |
124/247 |
| 🔴 | Functions | 59.63% | 65/109 |
| 🔴 | Lines | 54.1% (-0.43% 🔻) |
356/658 |
Show files with reduced coverage 🔻
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | ... / fetchResources.ts |
8.57% (-1.43% 🔻) |
0% | 0% | 8.82% (-1.52% 🔻) |
| 🔴 | ... / fetchActionPlan.ts |
3.85% (-0.08% 🔻) |
0% | 0% | 4% (-0.08% 🔻) |
Test suite run success
78 tests passing in 7 suites.
Report generated by 🧪jest coverage report action from 6a0f175
…ersion_id and suffix in conjunction
There was a problem hiding this comment.
Pull request overview
This pull request adds the ability to use a suffix query parameter to select different prompt templates for the referral generation feature. This enables the system to support region-specific prompts (e.g., Austin vs. Keystone area resources).
- Adds
suffixparameter throughout the frontend-to-backend pipeline for generate-referrals - Implements prompt name construction logic that appends the suffix to the base prompt name
- Adds new prompt version configuration for "generate_referrals_keystone"
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/util/fetchResources.ts |
Adds optional suffix parameter and includes it in API request body when provided |
frontend/src/util/fetchActionPlan.ts |
Refactors request body construction (code style improvement, unrelated to feature) |
frontend/src/components/ResourcesList.tsx |
Formatting change to improve code readability (style improvement, unrelated to feature) |
frontend/src/app/[locale]/generate-referrals/page.tsx |
Extracts suffix from URL query parameters and passes it to fetchResources |
app/src/pipelines/generate_referrals/pipeline_wrapper.py |
Adds suffix parameter to run_api and _run methods, updates error message |
app/src/pipelines/generate_action_plan/pipeline_wrapper.py |
Refactors to pass prompt template at runtime rather than during setup |
app/src/common/haystack_utils.py |
Implements core logic to append suffix to prompt name when constructing full prompt identifier |
app/src/app_config.py |
Adds "generate_referrals_keystone" prompt version configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise HTTPException( | ||
| status_code=422, | ||
| detail=f"The requested prompt version '{prompt_version_id}' could not be retrieved due to HTTP status {he.response.status_code}", | ||
| detail=f"The requested prompt version '{prompt_version_id}' with suffix '{suffix}' could not be retrieved due to HTTP status {he.response.status_code}", |
There was a problem hiding this comment.
The error message will display "with suffix ''" when no suffix is provided, which could be confusing to users. Consider conditionally including the suffix part only when a suffix is actually provided, for example: f"The requested prompt version '{prompt_version_id}'{f' with suffix '{suffix}'' if suffix else ''} could not be retrieved..."
| detail=f"The requested prompt version '{prompt_version_id}' with suffix '{suffix}' could not be retrieved due to HTTP status {he.response.status_code}", | |
| detail=( | |
| f"The requested prompt version '{prompt_version_id}'" | |
| f"{f\" with suffix '{suffix}'\" if suffix else ''} " | |
| f"could not be retrieved due to HTTP status {he.response.status_code}" | |
| ), |
| def get_phoenix_prompt( | ||
| prompt_name: str, prompt_version_id: str = "", suffix: str = "" | ||
| ) -> list[ChatMessage]: | ||
| full_prompt_name = f"{prompt_name}_{suffix}" if suffix else prompt_name | ||
| prompt_ver = phoenix_utils.get_prompt_template(full_prompt_name, prompt_version_id) | ||
| return to_chat_messages(prompt_ver._template["messages"]) |
There was a problem hiding this comment.
The new suffix parameter handling in the get_phoenix_prompt function lacks test coverage. Consider adding tests to verify that the suffix is correctly appended to the prompt name when provided, and that the function behaves correctly when suffix is empty or not provided.
This reverts commit bd38d76.
Ticket
https://navalabs.atlassian.net/browse/DST-1403
Changes
added the ability to set a suffix to use different prompts
Context for reviewers
https://nava.slack.com/archives/C06ETE82UHM/p1767839825584939
See latest in https://nava.slack.com/archives/C06ETE82UHM/p1762200108857569
Testing
Use
http://localhost:3001/generate-referrals?suffix=keystone
Preview environment for frontend
Preview environment for app
♻️ Environment destroyed ♻️