fix(deriver): document explicit observation JSON shape#901
fix(deriver): document explicit observation JSON shape#901PraveenKumarSridhar wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe deriver prompt and representation schema now require explicit observations as objects with ChangesExplicit observation shape
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/representation.py`:
- Line 117: Reformat the description argument in the relevant configuration or
declaration so the long string is split across adjacent concatenated string
literals, keeping the exact text unchanged and each Python source line within
the 88-character limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ac18d8b-e8b7-43e6-8d1d-fd09243e319a
📒 Files selected for processing (4)
src/deriver/prompts.pysrc/utils/representation.pytests/deriver/test_prompts.pytests/llm/test_backends/test_openai.py
|
|
||
| explicit: list[ExplicitObservationBase] = Field( | ||
| description="Facts LITERALLY stated by the user - direct quotes or clear paraphrases only, no interpretation or inference. Example: ['The user is 25 years old', 'The user has a dog named Rover']", | ||
| description='Facts LITERALLY stated by the user - direct quotes or clear paraphrases only, no interpretation or inference. Example: [{"content": "The user is 25 years old"}, {"content": "The user has a dog named Rover"}]', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Line 117 exceeds the 88-character limit.
As per coding guidelines, Python lines should be kept within 88 characters to match Black-compatible formatting. The description string on this line is well over that limit. Consider splitting the string across multiple concatenated lines.
♻️ Proposed refactor
- explicit: list[ExplicitObservationBase] = Field(
- description='Facts LITERALLY stated by the user - direct quotes or clear paraphrases only, no interpretation or inference. Example: [{"content": "The user is 25 years old"}, {"content": "The user has a dog named Rover"}]',
+ explicit: list[ExplicitObservationBase] = Field(
+ description=(
+ "Facts LITERALLY stated by the user - direct quotes or clear "
+ "paraphrases only, no interpretation or inference. Example: "
+ '[{"content": "The user is 25 years old"}, '
+ '{"content": "The user has a dog named Rover"}]'
+ ),
default_factory=list,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description='Facts LITERALLY stated by the user - direct quotes or clear paraphrases only, no interpretation or inference. Example: [{"content": "The user is 25 years old"}, {"content": "The user has a dog named Rover"}]', | |
| explicit: list[ExplicitObservationBase] = Field( | |
| description=( | |
| "Facts LITERALLY stated by the user - direct quotes or clear " | |
| "paraphrases only, no interpretation or inference. Example: " | |
| '[{"content": "The user is 25 years old"}, ' | |
| '{"content": "The user has a dog named Rover"}]' | |
| ), | |
| default_factory=list, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/representation.py` at line 117, Reformat the description argument
in the relevant configuration or declaration so the long string is split across
adjacent concatenated string literals, keeping the exact text unchanged and each
Python source line within the 88-character limit.
Source: Coding guidelines
6b45815 to
4112f94
Compare
|
Fixed in 4112f94 — split the description string across adjacent concatenated literals in src/utils/representation.py:117 to satisfy the 88-char line limit. Schema text unchanged; tests still 55/55. Thanks for the catch, CodeRabbit! |
|
Hi maintainers — I noticed that #893 was closed after #907 added schema injection for The prompt and schema-description clarifications in this PR are not currently in I am happy to rebase and update it if you would still like these changes merged. |
|
Finn-loop review of 4112f94 CI: not configured — no required checks reported on this branch ReviewSummary: Updates the deriver prompt and PromptRepresentation schema to use object-shaped examples ( 1. Must fix before merge
2. Should fix soonNone. 3. Safe to mergeYes on code merits — the changes are minimal, exactly scoped to the contract, and well-tested. The stale branch-protection rule and missing CI are repo-configuration issues, not code defects. Note: No formal issue was linked ( Labels that should be applied: |
2 participants
Summary
PromptRepresentation.explicitJSON shape forjson_objectstructured-output fallback.PromptRepresentationschema description so OpenAI-compatible providers see object-shaped examples, not bare string examples.json_objectschema-injection path.Closes #893.
Why
PromptRepresentation.explicitexpects a list of objects with acontentfield. Injson_objectmode, providers rely on prompt/schema text to infer the shape, so examples that imply bare strings can lead to validation failures and silently empty observations.Testing
.venv/bin/pytest tests/deriver/test_prompts.py tests/llm/test_backends/test_openai.py -q→ 55 passed.venv/bin/ruff check src/deriver/prompts.py src/utils/representation.py tests/deriver/test_prompts.py tests/llm/test_backends/test_openai.py→ All checks passed.venv/bin/basedpyright src/deriver/prompts.py src/utils/representation.py tests/deriver/test_prompts.py tests/llm/test_backends/test_openai.py→ 0 errorsBroader deriver queue tests were attempted, but local Postgres auth setup rejected the test DB connection, so I kept verification to the prompt and OpenAI backend tests touched by this fix.
Developed with AI assistance; reproduced, tested, and reviewed locally before submission.
Summary by CodeRabbit
Improvements
contentfield.Bug Fixes
Tests
json_objectresponses using the prompt representation.