Skip to content

fix: validate unsupported template placeholders - #249

Merged
mamayer19 merged 3 commits into
superlinked:mainfrom
RoopakBRK:fix/template-placeholder-validation
Sep 7, 2026
Merged

fix: validate unsupported template placeholders#249
mamayer19 merged 3 commits into
superlinked:mainfrom
RoopakBRK:fix/template-placeholder-validation

Conversation

@RoopakBRK

@RoopakBRK RoopakBRK commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

This PR adds a small validation improvement for query and document templates.

Issue - When an unsupported placeholder is used in a template, for example {domain}, Python currently raises a KeyError when formatting the template. This doesn't make it very clear to the user that the problem is with the template they provided.

What did I change ?
I added handling for the KeyError raised by template.format() and converted it into a ValueError with a more helpful message that points out which placeholder is not supported.

Testing
I ran the targeted tests for extract_texts, and all 9 tests passed. I also ran:
mise run test
mise run lint
mise run typecheck
git diff --check

File Changed -> sie/packages/sie_server/src/sie_server/adapters/_utils.py

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling when a template contains an unsupported placeholder.
    • Error messages now identify the problematic placeholder, making template issues easier to diagnose.
    • Original error details are retained to support troubleshooting and clarify the underlying formatting failure.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 6a286a13-8603-4e1b-b341-6b1e6f1388eb

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc0870 and 367e2fc.

📒 Files selected for processing (1)
  • packages/sie_server/src/sie_server/adapters/_utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/sie_server/src/sie_server/adapters/_utils.py

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The template text extraction path now reports unsupported placeholders with a descriptive ValueError and preserves the original KeyError.

Changes

Template error handling

Layer / File(s) Summary
Unsupported placeholder reporting
packages/sie_server/src/sie_server/adapters/_utils.py
extract_texts converts missing template placeholders into a descriptive ValueError and chains the original KeyError.

Merge Risk: ⚪ Minimal · up to 367e2

Unsupported template placeholders now produce a clear validation error instead of an unhandled missing-key failure. No concrete merge-blocking risk is currently identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: validating unsupported template placeholders.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 2, 2026
@RoopakBRK
RoopakBRK requested a review from a team as a code owner September 4, 2026 14:32
@mamayer19

Copy link
Copy Markdown
Contributor

Hi @RoopakBRK, thank you for the contribution! Sorry for the radio silence, we've been revamping our internal workflows that delayed a response on this PR. We can pick this up for a review early next week.

@mamayer19
mamayer19 merged commit 0508e17 into superlinked:main Sep 7, 2026
14 checks passed
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