feat(assessment): Configuration & Versioning with Batch API Support - #1097
Open
vprashrex wants to merge 18 commits into
Open
feat(assessment): Configuration & Versioning with Batch API Support#1097vprashrex wants to merge 18 commits into
vprashrex wants to merge 18 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds assessment configuration models, UUID-based persistence, a gated assessment submission API, staged provider batch processing, asynchronous Celery execution, webhook callbacks, structured results, and compatibility updates for legacy assessment flows. ChangesAssessment platform
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
github-actions
Bot
changed the title
vprashrex
changed the title
Prajna1999
approved these changes
Jul 31, 2026
- Added `callbacks.py` to handle webhook delivery for assessment results. - Introduced `results.py` to assemble `AssessmentBatchResult` for BATCH submissions. - Created `submission.py` for handling BATCH assessment submissions, including config resolution and input validation. - Updated `service.py` to accommodate changes in config reference handling. - Enhanced `mappers.py` to support JSON schema for assessment pipelines. - Modified tests to reflect changes in config reference structure. - Updated documentation to clarify assessment module structure and functionality.
OpenAPI changes 🔴 101 breaking changesCaution Downstream consumers may need an update before merging. Breaking changes ·
|
| Method | Path | Change | |
|---|---|---|---|
| 🔴 | GET |
/api/v1/assessment/assessments |
the response property data/anyOf[subschema #1]/items/experiment_name became optional for the status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
response property data/anyOf[subschema #1]/items/experiment_name list-of-types was widened by adding types null to media type application/json of response 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
the data/anyOf[subschema #1]/items/id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the required property data/anyOf[subschema #1]/items/dataset_id from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
for the path request parameter assessment_id, the type/format was changed from integer to string/uuid |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
the response property data/anyOf[subschema #1: AssessmentPublic]/experiment_name became optional for the status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
response property data/anyOf[subschema #1: AssessmentPublic]/experiment_name list-of-types was widened by adding types null to media type application/json of response 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
the data/anyOf[subschema #1: AssessmentPublic]/id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the required property data/anyOf[subschema #1: AssessmentPublic]/dataset_id from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id}/results |
for the path request parameter assessment_id, the type/format was changed from integer to string/uuid |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
for the path request parameter assessment_id, the type/format was changed from integer to string/uuid |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
the data/anyOf[subschema #1: AssessmentResponse]/assessment_id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/dataset_id from the response with the 200 status |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/dataset_name from the response with the 200 status |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/experiment_name from the response with the 200 status |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/num_configs from the response with the 200 status |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/runs from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
query request parameter assessment_id list-of-types was narrowed by removing types integer |
| 🔴 | GET |
/api/v1/assessment/runs |
for the query request parameter assessment_id, the type/format of property anyOf[subschema #1]/ was narrowed from integer to string/uuid |
| 🔴 | GET |
/api/v1/assessment/runs |
the data/anyOf[subschema #1]/items/assessment_id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | POST |
/api/v1/assessment/runs |
added the new required request property configs/items/id |
| 🔴 | POST |
/api/v1/assessment/runs |
added the new required request property configs/items/version |
| 🔴 | POST |
/api/v1/assessment/runs |
added the new required request property input_binding |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
the data/anyOf[subschema #1: AssessmentRunPublic]/assessment_id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
the data/anyOf[subschema #1: AssessmentRunPublic]/assessment_id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the optional property data/anyOf[subschema #1]/items/counts from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the optional property data/anyOf[subschema #1]/items/dataset_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the optional property data/anyOf[subschema #1]/items/error_message from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the optional property data/anyOf[subschema #1]/items/run_stats from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new COMPLETED enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new FAILED enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new PENDING enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new PROCESSING enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the optional property data/anyOf[subschema #1: AssessmentPublic]/counts from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the optional property data/anyOf[subschema #1: AssessmentPublic]/dataset_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the optional property data/anyOf[subschema #1: AssessmentPublic]/error_message from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the optional property data/anyOf[subschema #1: AssessmentPublic]/run_stats from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new COMPLETED enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new FAILED enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new PENDING enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new PROCESSING enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/dataset_id from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/dataset_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/experiment_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/input from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/pipeline from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/prefilter_total_passed from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/prefilter_total_rejected from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/prefilter_total_rows from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/stage from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/stage_status from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new COMPLETED enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new FAILED enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new PENDING enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new PROCESSING enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property attachments |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property configs/items/config_id |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property configs/items/config_version |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property output_schema |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property prefilter_config |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property prompt_template |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property system_instruction |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property text_columns |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/dataset_id from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/dataset_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/experiment_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/input from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/pipeline from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_passed from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_rejected from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_rows from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/stage from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/stage_status from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new COMPLETED enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new FAILED enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new PENDING enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new PROCESSING enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/dataset_id from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/dataset_name from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/experiment_name from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/input from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/pipeline from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_passed from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_rejected from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_rows from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/stage from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/stage_status from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new COMPLETED enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new FAILED enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new PENDING enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new PROCESSING enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/configs/{config_id} |
removed the request property tag |
| 🔴 | GET |
/api/v1/configs/{config_id}/versions |
deleted the query request parameter tag |
| 🔴 | POST |
/api/v1/configs/{config_id}/versions |
deleted the query request parameter tag |
| 🔴 | DELETE |
/api/v1/configs/{config_id}/versions/{version_number} |
deleted the query request parameter tag |
| 🔴 | GET |
/api/v1/configs/{config_id}/versions/{version_number} |
deleted the query request parameter tag |
Full changelog · 121
| Method | Path | Change | |
|---|---|---|---|
| 🔴 | GET |
/api/v1/assessment/assessments |
the response property data/anyOf[subschema #1]/items/experiment_name became optional for the status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
response property data/anyOf[subschema #1]/items/experiment_name list-of-types was widened by adding types null to media type application/json of response 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
the data/anyOf[subschema #1]/items/id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the required property data/anyOf[subschema #1]/items/dataset_id from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
for the path request parameter assessment_id, the type/format was changed from integer to string/uuid |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
the response property data/anyOf[subschema #1: AssessmentPublic]/experiment_name became optional for the status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
response property data/anyOf[subschema #1: AssessmentPublic]/experiment_name list-of-types was widened by adding types null to media type application/json of response 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
the data/anyOf[subschema #1: AssessmentPublic]/id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the required property data/anyOf[subschema #1: AssessmentPublic]/dataset_id from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id}/results |
for the path request parameter assessment_id, the type/format was changed from integer to string/uuid |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
for the path request parameter assessment_id, the type/format was changed from integer to string/uuid |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
the data/anyOf[subschema #1: AssessmentResponse]/assessment_id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/dataset_id from the response with the 200 status |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/dataset_name from the response with the 200 status |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/experiment_name from the response with the 200 status |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/num_configs from the response with the 200 status |
| 🔴 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
removed the required property data/anyOf[subschema #1: AssessmentResponse]/runs from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
query request parameter assessment_id list-of-types was narrowed by removing types integer |
| 🔴 | GET |
/api/v1/assessment/runs |
for the query request parameter assessment_id, the type/format of property anyOf[subschema #1]/ was narrowed from integer to string/uuid |
| 🔴 | GET |
/api/v1/assessment/runs |
the data/anyOf[subschema #1]/items/assessment_id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | POST |
/api/v1/assessment/runs |
added the new required request property configs/items/id |
| 🔴 | POST |
/api/v1/assessment/runs |
added the new required request property configs/items/version |
| 🔴 | POST |
/api/v1/assessment/runs |
added the new required request property input_binding |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
the data/anyOf[subschema #1: AssessmentRunPublic]/assessment_id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
the data/anyOf[subschema #1: AssessmentRunPublic]/assessment_id response's property type/format changed from integer to string/uuid for status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the optional property data/anyOf[subschema #1]/items/counts from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the optional property data/anyOf[subschema #1]/items/dataset_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the optional property data/anyOf[subschema #1]/items/error_message from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments |
removed the optional property data/anyOf[subschema #1]/items/run_stats from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new COMPLETED enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new FAILED enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new PENDING enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments |
added the new PROCESSING enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the optional property data/anyOf[subschema #1: AssessmentPublic]/counts from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the optional property data/anyOf[subschema #1: AssessmentPublic]/dataset_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the optional property data/anyOf[subschema #1: AssessmentPublic]/error_message from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
removed the optional property data/anyOf[subschema #1: AssessmentPublic]/run_stats from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new COMPLETED enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new FAILED enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new PENDING enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the new PROCESSING enum value to the data/anyOf[subschema #1: AssessmentPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/dataset_id from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/dataset_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/experiment_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/input from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/pipeline from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/prefilter_total_passed from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/prefilter_total_rejected from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/prefilter_total_rows from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/stage from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
removed the optional property data/anyOf[subschema #1]/items/stage_status from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new COMPLETED enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new FAILED enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new PENDING enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs |
added the new PROCESSING enum value to the data/anyOf[subschema #1]/items/status response property for the response status 200 |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property attachments |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property configs/items/config_id |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property configs/items/config_version |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property output_schema |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property prefilter_config |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property prompt_template |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property system_instruction |
| 🔴 | POST |
/api/v1/assessment/runs |
removed the request property text_columns |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/dataset_id from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/dataset_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/experiment_name from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/input from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/pipeline from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_passed from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_rejected from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_rows from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/stage from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/stage_status from the response with the 200 status |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new COMPLETED enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new FAILED enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new PENDING enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | GET |
/api/v1/assessment/runs/{run_id} |
added the new PROCESSING enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/dataset_id from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/dataset_name from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/experiment_name from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/input from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/pipeline from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_passed from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_rejected from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/prefilter_total_rows from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/stage from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
removed the optional property data/anyOf[subschema #1: AssessmentRunPublic]/stage_status from the response with the 200 status |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new COMPLETED enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new COMPLETED_WITH_ERRORS enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new FAILED enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new PENDING enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the new PROCESSING enum value to the data/anyOf[subschema #1: AssessmentRunPublic]/status response property for the response status 200 |
| 🔴 | PATCH |
/api/v1/configs/{config_id} |
removed the request property tag |
| 🔴 | GET |
/api/v1/configs/{config_id}/versions |
deleted the query request parameter tag |
| 🔴 | POST |
/api/v1/configs/{config_id}/versions |
deleted the query request parameter tag |
| 🔴 | DELETE |
/api/v1/configs/{config_id}/versions/{version_number} |
deleted the query request parameter tag |
| 🔴 | GET |
/api/v1/configs/{config_id}/versions/{version_number} |
deleted the query request parameter tag |
| 🟢 | — |
— |
removed the schema AssessmentRunCounts |
| 🟢 | — |
— |
removed the schema AssessmentRunStat |
| 🟢 | — |
— |
a breaking change was detected but the version is still 0.5.0 |
| 🟢 | GET |
/api/v1/assessment/assessments |
added the optional property data/anyOf[subschema #1]/items/executions to the response with the 200 status |
| 🟢 | GET |
/api/v1/assessment/assessments/{assessment_id} |
added the optional property data/anyOf[subschema #1: AssessmentPublic]/executions to the response with the 200 status |
| 🟢 | POST |
/api/v1/assessment/assessments/{assessment_id}/retry |
added the optional property data/anyOf[subschema #1: AssessmentResponse]/executions to the response with the 200 status |
| 🟢 | GET |
/api/v1/assessment/runs |
added the optional property data/anyOf[subschema #1]/items/batch_job_id to the response with the 200 status |
| 🟢 | GET |
/api/v1/assessment/runs |
added the optional property data/anyOf[subschema #1]/items/execution to the response with the 200 status |
| 🟢 | POST |
/api/v1/assessment/runs |
the experiment_name request property's minLength was decreased from 1 to 0 |
| 🟢 | POST |
/api/v1/assessment/runs |
added #/components/schemas/AssessmentRunResponse to the data response property anyOf list for the response status 200 |
| 🟢 | POST |
/api/v1/assessment/runs |
removed #/components/schemas/AssessmentResponse from the data response property anyOf list for the response status 200 |
| 🟢 | GET |
/api/v1/assessment/runs/{run_id} |
added the optional property data/anyOf[subschema #1: AssessmentRunPublic]/batch_job_id to the response with the 200 status |
| 🟢 | GET |
/api/v1/assessment/runs/{run_id} |
added the optional property data/anyOf[subschema #1: AssessmentRunPublic]/execution to the response with the 200 status |
| 🟢 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the optional property data/anyOf[subschema #1: AssessmentRunPublic]/batch_job_id to the response with the 200 status |
| 🟢 | PATCH |
/api/v1/assessment/runs/{run_id}/post-processing |
added the optional property data/anyOf[subschema #1: AssessmentRunPublic]/execution to the response with the 200 status |
| 🟢 | POST |
/api/v1/assessment/runs/{run_id}/resume |
added #/components/schemas/AssessmentRunResponse to the data response property anyOf list for the response status 200 |
| 🟢 | POST |
/api/v1/assessment/runs/{run_id}/resume |
removed #/components/schemas/AssessmentResponse from the data response property anyOf list for the response status 200 |
| 🟢 | POST |
/api/v1/assessment/runs/{run_id}/retry |
added #/components/schemas/AssessmentRunResponse to the data response property anyOf list for the response status 200 |
| 🟢 | POST |
/api/v1/assessment/runs/{run_id}/retry |
removed #/components/schemas/AssessmentResponse from the data response property anyOf list for the response status 200 |
| 🟢 | POST |
/api/v1/assessments |
endpoint added |
main ↔ 487df991 · generated by oasdiff
…ma and pre-filter handling
…improve docstring clarity in batch stage parameters
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/services/assessment/service.py (2)
53-86: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftComplete the parent
InputBindingmigration.
AssessmentRunno longer storesinput, andcreate_assessment_runno longer acceptsassessment_input. The current call raisesTypeError. Removing only that keyword leavesAssessment.inputunset, so batch submission and retry have no parent binding.
backend/app/services/assessment/service.py#L53-L86: pass the parent assessment input to_build_retry_requestand read that binding instead offirst_run.input.backend/app/services/assessment/service.py#L118-L195: build the current{prompt, text_columns, attachments}binding, pass it asinput_bindingtocreate_assessment, and removeassessment_inputfromcreate_assessment_run.🤖 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 `@backend/app/services/assessment/service.py` around lines 53 - 86, The AssessmentRun InputBinding migration is incomplete, leaving retry and batch-created assessments without their parent input binding. In backend/app/services/assessment/service.py lines 53-86, update _build_retry_request to accept the parent assessment input and read it instead of first_run.input; in lines 118-195, build the current {prompt, text_columns, attachments} binding, pass it as input_binding to create_assessment, and remove assessment_input from create_assessment_run.
300-338: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse execution state when resuming a run.
AssessmentRunno longer definesstage_status,stage, orpipeline. This function raisesAttributeErrorbefore it can resume a failed run. It also writes the lowercase string"processing"instead ofAssessmentStatus.PROCESSING.Read and update
stage,stage_status, andpipelinethrough_read_execand_write_exec. Setrun.statustoAssessmentStatus.PROCESSING.🤖 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 `@backend/app/services/assessment/service.py` around lines 300 - 338, Update the resume flow around the failed-state and resumable-stage checks to read stage_status, stage, and pipeline via _read_exec, and write the reset stage_status and stage via _write_exec instead of accessing AssessmentRun attributes directly. Set run.status to AssessmentStatus.PROCESSING rather than the lowercase string, and use the execution-state stage value in the resume log.
🧹 Nitpick comments (15)
backend/app/models/config/assessment_blob.py (2)
58-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist the duplicated
stop_on_faildeclaration intoPreFilterBase.Both subclasses repeat the same field description word for word. Only the default differs. Declare the field once in
PreFilterBaseand override the default in each subclass.♻️ Proposed consolidation
class PreFilterBase(SQLModel): """Shared pre-filter fields — each pre-filter runs its own llm call.""" + + stop_on_fail: bool = Field( + default=False, + description=( + "If true, a failing verdict stops the chain and skips the assessment for " + "that item; if false, the verdict is just recorded and the assessment runs." + ), + )Then in
TopicRelevanceFilter, keep only the changed default:stop_on_fail: bool = Trueand remove the block from
DuplicateDetectionFilter.Also applies to: 81-87
🤖 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 `@backend/app/models/config/assessment_blob.py` around lines 58 - 64, Move the shared stop_on_fail field declaration and description into PreFilterBase. Remove the duplicated declaration from DuplicateDetectionFilter, and in TopicRelevanceFilter retain only a stop_on_fail annotation with its subclass-specific default, preserving the existing differing defaults.
18-24: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider validating
formatagainsttype.An
InputColumnwithtype: "text"andformat: "url"is accepted, and animageorformatis also accepted. The documentation inbackend/app/api/docs/config/create.md(lines 78-80) states thatformatapplies to attachment columns. A cross-field validator would reject the ambiguous combinations at config time instead of during batch assembly.♻️ Proposed validator
type: Literal["text", "image", "pdf"] = "text" strict: bool = False format: Literal["url", "base64"] | None = None + + `@model_validator`(mode="after") + def _validate_format(self) -> "InputColumn": + if self.type == "text" and self.format is not None: + raise ValueError("format is only valid for image or pdf columns") + if self.type in ("image", "pdf") and self.format is None: + raise ValueError(f"format is required for '{self.type}' columns") + return self🤖 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 `@backend/app/models/config/assessment_blob.py` around lines 18 - 24, Update InputColumn validation to enforce the relationship between type and format: text columns must not specify format, while image and pdf attachment columns must specify either url or base64. Add a cross-field validator on InputColumn that rejects these invalid combinations during configuration parsing and preserves the existing allowed values.backend/app/crud/config/config.py (1)
148-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTag scoping is now inconsistent across the CRUD surface.
delete_or_raiseand theget_configroute are tag-scoped.update_or_raise(line 126) still calls the unscopedexists_or_raise, andread_one(line 83) applies no tag filter. A caller can therefore rename an ASSESSMENT config through the default-scoped update path, while the same id returns 404 from get and delete.
read_oneis also the lookup used by_resolve_configinbackend/app/services/assessment/api/submission.py, which compensates with its own explicitparent.tag != ConfigTag.ASSESSMENTcheck. That compensation confirms the unscoped default is the weaker contract.Extend the tag parameter to
update_or_raisefor a consistent scope.♻️ Proposed change
- def update_or_raise(self, config_id: UUID, config_update: ConfigUpdate) -> Config: - config = self.exists_or_raise(config_id) + def update_or_raise( + self, + config_id: UUID, + config_update: ConfigUpdate, + tag: ConfigTag = ConfigTag.DEFAULT, + ) -> Config: + config = self.exists_in_tag_scope_or_raise(config_id, tag)Pass
tagfrom the update route in the same way asget_configanddelete_config.🤖 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 `@backend/app/crud/config/config.py` around lines 148 - 151, Extend update_or_raise to accept a ConfigTag parameter and use exists_in_tag_scope_or_raise for its lookup, preserving the default tag. Update the update route to pass its tag argument consistently with get_config and delete_config, while leaving read_one unchanged.backend/app/alembic/versions/076_refactor_assessment_tables.py (2)
182-191: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a database-level UUID default for
assessment.id.
Assessment.idusesdefault_factory=uuid4, so ORM-driven inserts get a value, but this migration creates the column as NOT NULL withoutserver_default=sa.text("gen_random_uuid()"). Add the database default to protect direct SQL inserts that omitid.🤖 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 `@backend/app/alembic/versions/076_refactor_assessment_tables.py` around lines 182 - 191, Add a database-level UUID server default to the assessment.id column definition in the migration, using gen_random_uuid() while preserving its non-null constraint and primary key configuration. Update the sa.Column declaration within the assessment table migration rather than changing ORM defaults.
348-349: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the PostgreSQL ENUM type for the named enum drop.
Keep the downgrade symmetric with the upgrade: drop
assessment_statusandassessment_methodusingpostgresql.ENUM(name=...)instead ofsa.Enum(name=...).🤖 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 `@backend/app/alembic/versions/076_refactor_assessment_tables.py` around lines 348 - 349, Update the downgrade enum cleanup to use PostgreSQL-specific ENUM objects for both ASSESSMENT_STATUS_ENUM and ASSESSMENT_METHOD_ENUM. Replace the sa.Enum constructors in these drop calls with postgresql.ENUM while preserving the existing names, bind, and checkfirst behavior.backend/app/services/assessment/api/batch.py (3)
573-599: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the poll outcome strings into an enum.
_poll_outcomereturns the bare literals"processing","completed", and"failed", andrun_batch_stagecompares against the same literals. A typo on either side silently changes control flow. The file already definesApiStageandStageKindfor this purpose. The coding guidelines forbid magic values and require repeated literals to become constants or enums.As per coding guidelines: "Do not use magic values; extract repeated literals into constants, enums, or settings."
♻️ Proposed refactor
+class PollOutcomeEnum(StrEnum): + PROCESSING = "processing" + COMPLETED = "completed" + FAILED = "failed" + + def _poll_outcome( session: Session, provider: BatchProvider, batch_job: BatchJob -) -> tuple[str, list[dict[str, Any]] | None]: +) -> tuple[PollOutcomeEnum, list[dict[str, Any]] | None]:Also applies to: 760-772
🤖 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 `@backend/app/services/assessment/api/batch.py` around lines 573 - 599, Introduce a dedicated enum for the poll outcomes processing, completed, and failed, then update _poll_outcome’s return annotation and all return values to use that enum. Replace the matching literal comparisons in run_batch_stage with the enum members, keeping the existing control flow unchanged.Source: Coding guidelines
507-510: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
stageparameter.Ruff reports ARG001 for
stage. The body uses onlybag,kind, andtotal. Drop the parameter and update the single call site at line 528.🤖 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 `@backend/app/services/assessment/api/batch.py` around lines 507 - 510, Remove the unused stage parameter from _row_subset, then update its single call site to pass only bag, kind, and total while preserving the existing subset-selection behavior.Source: Linters/SAST tools
206-221: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNarrow the pre-filter annotations from
Any.
duplicate_detection.contentis defined, so_stage_promptis correct. Update_prefilter_for_stageandbuild_pipelineto return the concrete pre-filter model type instead ofAny; the coding guidelines require narrow type hints for every parameter and return value, and-> Anymust not stand in for a specific annotation.🤖 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 `@backend/app/services/assessment/api/batch.py` around lines 206 - 221, Update _prefilter_for_stage to return the concrete pre-filter model type from blob.pre_filters instead of Any, using the existing pre.topic_relevance and pre.duplicate_detection branches to keep the stage-specific behavior intact. Then adjust build_pipeline to consume that narrowed return type and propagate the same concrete annotation through its parameter and return signatures, replacing any remaining Any-based pre-filter typing with the specific model type already represented in the config objects.Source: Coding guidelines
backend/app/services/llm/mappers.py (3)
57-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNarrow the
dictannotations on the three new helpers.
_ensure_openai_strict_schema,_strip_additional_properties, and_convert_json_schema_to_googleannotate their parameter and return value as baredict. Usedict[str, Any]. The coding guidelines require narrow type hints for every parameter and return value.As per coding guidelines: "provide narrow type hints for every function parameter and return value."
Also applies to: 79-79, 102-102
🤖 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 `@backend/app/services/llm/mappers.py` at line 57, Update the annotations on _ensure_openai_strict_schema, _strip_additional_properties, and _convert_json_schema_to_google to use dict[str, Any] for both parameters and return values, preserving their existing behavior and ensuring Any is available in the module imports.Source: Coding guidelines
79-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_strip_additional_propertiesskips composition keywords.The recursion covers
propertiesanditemsonly. AnadditionalPropertieskey inside ananyOf,oneOf,allOf, or$defsbranch survives and reachest_schema. Extend the traversal to match the fix in_ensure_openai_strict_schema, or extract one shared walker that both functions use with different per-node transforms.🤖 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 `@backend/app/services/llm/mappers.py` around lines 79 - 99, Update _strip_additional_properties to recursively traverse schemas nested under anyOf, oneOf, allOf, and $defs, removing additionalProperties from every branch while preserving the existing properties and items handling. Reuse the traversal approach from _ensure_openai_strict_schema or extract a shared walker with the functions’ existing node-specific transformations.
20-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid relying on Google’s private schema transformer.
google.genai._transformersis private, and the strict dependency range allows compatible changes. Wrap thet_schemacall inbackend/app/services/llm/mappers.py:102so_convert_json_schema_to_googlecan continue withnormalized_schemaif the SDK changes, and tighten the upper bound ofgoogle-genaiinbackend/pyproject.toml.🤖 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 `@backend/app/services/llm/mappers.py` at line 20, Update _convert_json_schema_to_google to wrap the genai_transformers.t_schema call in compatibility handling and fall back to normalized_schema if the private transformer is unavailable or changes behavior; also tighten the google-genai upper dependency bound in pyproject.toml to a tested compatible range.backend/app/models/assessment/assessment.py (1)
36-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffEnum class names miss the required
Enumsuffix.The coding guidelines require an
Enumsuffix for enum class names.AssessmentMethod,AssessmentStatus,Stage, andStageStatusdo not use it. If these names are already established across the codebase, keep them and note the exception, because a rename touches the migration enum names, CRUD, services, and tests.As per coding guidelines: "Use an
Enumsuffix for enum class names."🤖 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 `@backend/app/models/assessment/assessment.py` around lines 36 - 42, Rename the enum classes AssessmentMethod, AssessmentStatus, Stage, and StageStatus to AssessmentMethodEnum, AssessmentStatusEnum, StageEnum, and StageStatusEnum, updating all references across migrations, CRUD, services, and tests. If these names are intentionally established and cannot be renamed safely, preserve them and document the guideline exception.Source: Coding guidelines
backend/app/services/assessment/api/results.py (1)
112-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
bag.get("input_schema")never resolves.No writer places
input_schemain the execution bag.backend/app/services/assessment/api/batch.pyline 526 reads it fromblob.assessment.params, not from the bag.input_columnsis therefore always{}here, andbuild_rowsclassifies every column as text.The current code is not affected, because
build_resultdiscards both returned lists and uses onlylen(rows). The read is misleading and becomes a defect if the classification is used later. Read the schema from the config blob, or drop the argument and add a short comment stating that only the row count is needed.🤖 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 `@backend/app/services/assessment/api/results.py` around lines 112 - 117, Update the result-building flow around build_rows to stop reading input_schema from the execution bag, since that key is never written there. Either obtain the schema from blob.assessment.params, as in the batch flow, or remove the schema dependency when only len(rows) is used and document that purpose with a brief comment.backend/app/celery/tasks/job_execution.py (1)
351-360: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare the task contract.
Add narrow annotations for every parameter and the return value of
run_assessment_api_batch. Replacepriority=2with a named task-priority constant.As per coding guidelines, “Use Python 3.11+ and provide narrow type hints for every function parameter and return value” and “Do not use magic values.”
🤖 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 `@backend/app/celery/tasks/job_execution.py` around lines 351 - 360, Update run_assessment_api_batch to provide narrow type annotations for self, execution_id, organization_id, project_id, trace_id, kwargs, and its return value. Replace the literal priority=2 with the appropriate named task-priority constant already used by the task configuration.Source: Coding guidelines
backend/app/crud/assessment/api.py (1)
29-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDefine narrow persisted payload types.
inputandstateusedict[str, Any]. These values are persisted and later consumed by the batch driver. Define a JSON value type for generic stored input and aTypedDictfor the execution bag fields.As per coding guidelines, “Use Python 3.11+ and provide narrow type hints for every function parameter and return value.”
Also applies to: 108-110
🤖 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 `@backend/app/crud/assessment/api.py` around lines 29 - 36, Replace the broad dict[str, Any] annotations for persisted input and state in create_assessment and the related execution-bag code with a reusable recursive JSON value type for generic stored input and a TypedDict describing the execution bag fields. Update every affected function parameter and return annotation while preserving the existing payload structure and behavior.Source: Coding guidelines
🤖 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 `@backend/app/api/docs/config/create.md`:
- Around line 81-86: Update the pre_filters documentation to explicitly state
that topic_relevance.stop_on_fail defaults to true and
duplicate_detection.stop_on_fail defaults to false. Replace the internal
TextLLMParams reference with its user-facing field list, including model and
temperature as applicable.
In `@backend/app/api/routes/config/config.py`:
- Line 100: In backend/app/api/routes/config/config.py at lines 100-100 and
155-155, add narrow return annotations to the get_config and delete_config
endpoint signatures: use APIResponse[ConfigPublic] for get_config and
APIResponse[Message] for delete_config, matching the existing list_configs
annotation style.
- Around line 93-99: Keep by-id config CRUD operations unscoped: update the GET
endpoint’s tag handling near the ConfigTag query declaration and the
corresponding PATCH endpoint near its tag parameter so reads and updates use the
same by-id behavior. Ensure delete_or_raise(config_id) and related by-id
operations do not default to ConfigTag.DEFAULT or require an explicit assessment
tag, preserving consistent access to either config type.
In `@backend/app/crud/assessment/core.py`:
- Around line 318-346: Update compute_run_counts and derive_assessment_status so
completed-with-errors child runs remain distinguishable from normally completed
runs. Ensure a parent with only COMPLETED_WITH_ERRORS children derives
COMPLETED_WITH_ERRORS, while preserving COMPLETED for all normally completed
children and the existing mixed-status behavior.
In `@backend/app/crud/model_config.py`:
- Around line 156-163: Update validate_blob_completion_models to validate every
enabled pre-filter completion’s flt.params["model"] using the existing
_validate_completion_model_or_raise path before saving, while retaining the
current assessment/completion validation. Ensure missing pre-filter models
produce the established ValueError/_fail behavior instead of a later KeyError.
In `@backend/app/models/config/assessment_blob.py`:
- Around line 9-11: Implement the documented shallow validation for
json_output_schema in AssessmentTextParams: require a non-empty dictionary whose
type value is JSON_SCHEMA_OBJECT_TYPE ("object"), rejecting empty schemas and
non-object schemas during configuration. Reuse the existing constant and ensure
invalid values fail before reaching the batch path.
- Around line 30-37: Update validate_blob_completion_models to traverse
blob.pre_filters.topic_relevance and blob.pre_filters.duplicate_detection,
validating each pre-filter’s explicit provider/model pair against model_config
before the existing assessment and completion validation runs. Reuse the same
provider/model validation logic already applied to blob.assessment and
blob.completion, while preserving current behavior for absent pre-filters.
- Around line 39-45: Add narrow return type annotations to both
`_validate_prefilter_params` and `validate_params`, using the enclosing model’s
self type so each `@model_validator(mode="after")` method explicitly declares
that it returns the validated model instance.
- Around line 13-15: Update DEFAULT_PREFILTER_MODEL in the pre-filter
configuration alongside DEFAULT_PREFILTER_PROVIDER to use the existing
registered OpenAI default model string instead of "gpt-5.6-luna". Keep
_validate_prefilter_params behavior unchanged so omitted params.model values
resolve to that registered default.
In `@backend/app/services/assessment/api/batch.py`:
- Around line 228-244: Update `_stage_params` to store both assessment
`json_output_schema` and `PREFILTER_VERDICT_SCHEMA` under `json_schema`,
matching the keys consumed by the OpenAI and Google mappers. Add a test
asserting that mapped OpenAI pre-filter parameters contain `text.format.schema`
equal to `PREFILTER_VERDICT_SCHEMA`.
- Around line 286-295: Update the Google-provider branch in
_submit_provider_batch to pass CompletionType.TEXT to
map_kaapi_to_google_params, adding the required import. Widen exception handling
around both _submit_stage calls in run_batch_stage so TypeError and other
programming errors invoke _fail, transition execution to FAILED, and trigger the
failure callback.
- Around line 742-778: Record the provider selected by _stage_provider_model in
the stage-specific bag state during _submit_stage. In run_batch_stage, use the
provider stored for the current stage when calling _build_batch_provider and
parse_batch_results instead of bag["provider"], preserving the existing
per-stage behavior for pre-filters and assessment.
In `@backend/app/services/assessment/api/callbacks.py`:
- Around line 42-58: Update the callback delivery flow around deliver and its
callers _finalize and _fail so a false result from send_callback is not silently
discarded after terminal batch execution. Persist a retryable delivery record or
execution failure state, and process it with retry/backoff before marking the
callback dropped; preserve the existing success path and callback payload.
In `@backend/app/services/assessment/api/results.py`:
- Around line 83-100: Update _load_assessment_outputs in results.py to stop
swallowing storage/read exceptions and propagate the failure (or return an error
marker) instead of returning an empty dict. Then adjust build_result to use that
failure signal so gate-passed rows get a populated error and _finalize in
batch.py can derive a non-COMPLETED status when assessment output cannot be
read.
In `@backend/app/services/assessment/api/submission.py`:
- Around line 122-164: Make the BATCH handoff in submit() atomic and durable:
wrap assessment, execution, state, and status writes in one service-owned
transaction, using non-committing flush/update operations in the CRUD helpers
create_assessment, create_execution, save_execution_state, and update_status.
Persist a durable task-intent/outbox record before commit and publish
run_assessment_api_batch only through that mechanism, with recovery for unqueued
PROCESSING runs. Add request-identity idempotency so retries return the existing
submission without recomputing prefilters or ordering; update the related
job_execution task flow to support the durable handoff.
In `@backend/app/services/llm/mappers.py`:
- Around line 57-76: The _ensure_openai_strict_schema function must ensure every
object property is listed in required before sending the schema to OpenAI. When
normalized contains properties, preserve any existing required entries and add
all property names, including when required is missing or empty, while retaining
the existing recursive normalization and additionalProperties behavior.
In `@docs/wiki/modules/assessment.md`:
- Around line 3-27: Update the assessment module documentation by removing “No
deep-dive doc yet.” and correcting the ownership descriptions so processing,
batch, and cron are attributed only to their CRUD layer under crud/assessment/,
while services/assessment/ lists only its actual service modules. Preserve the
documented distinction between the legacy RUN service pipeline and CRUD modules.
---
Outside diff comments:
In `@backend/app/services/assessment/service.py`:
- Around line 53-86: The AssessmentRun InputBinding migration is incomplete,
leaving retry and batch-created assessments without their parent input binding.
In backend/app/services/assessment/service.py lines 53-86, update
_build_retry_request to accept the parent assessment input and read it instead
of first_run.input; in lines 118-195, build the current {prompt, text_columns,
attachments} binding, pass it as input_binding to create_assessment, and remove
assessment_input from create_assessment_run.
- Around line 300-338: Update the resume flow around the failed-state and
resumable-stage checks to read stage_status, stage, and pipeline via _read_exec,
and write the reset stage_status and stage via _write_exec instead of accessing
AssessmentRun attributes directly. Set run.status to AssessmentStatus.PROCESSING
rather than the lowercase string, and use the execution-state stage value in the
resume log.
---
Nitpick comments:
In `@backend/app/alembic/versions/076_refactor_assessment_tables.py`:
- Around line 182-191: Add a database-level UUID server default to the
assessment.id column definition in the migration, using gen_random_uuid() while
preserving its non-null constraint and primary key configuration. Update the
sa.Column declaration within the assessment table migration rather than changing
ORM defaults.
- Around line 348-349: Update the downgrade enum cleanup to use
PostgreSQL-specific ENUM objects for both ASSESSMENT_STATUS_ENUM and
ASSESSMENT_METHOD_ENUM. Replace the sa.Enum constructors in these drop calls
with postgresql.ENUM while preserving the existing names, bind, and checkfirst
behavior.
In `@backend/app/celery/tasks/job_execution.py`:
- Around line 351-360: Update run_assessment_api_batch to provide narrow type
annotations for self, execution_id, organization_id, project_id, trace_id,
kwargs, and its return value. Replace the literal priority=2 with the
appropriate named task-priority constant already used by the task configuration.
In `@backend/app/crud/assessment/api.py`:
- Around line 29-36: Replace the broad dict[str, Any] annotations for persisted
input and state in create_assessment and the related execution-bag code with a
reusable recursive JSON value type for generic stored input and a TypedDict
describing the execution bag fields. Update every affected function parameter
and return annotation while preserving the existing payload structure and
behavior.
In `@backend/app/crud/config/config.py`:
- Around line 148-151: Extend update_or_raise to accept a ConfigTag parameter
and use exists_in_tag_scope_or_raise for its lookup, preserving the default tag.
Update the update route to pass its tag argument consistently with get_config
and delete_config, while leaving read_one unchanged.
In `@backend/app/models/assessment/assessment.py`:
- Around line 36-42: Rename the enum classes AssessmentMethod, AssessmentStatus,
Stage, and StageStatus to AssessmentMethodEnum, AssessmentStatusEnum, StageEnum,
and StageStatusEnum, updating all references across migrations, CRUD, services,
and tests. If these names are intentionally established and cannot be renamed
safely, preserve them and document the guideline exception.
In `@backend/app/models/config/assessment_blob.py`:
- Around line 58-64: Move the shared stop_on_fail field declaration and
description into PreFilterBase. Remove the duplicated declaration from
DuplicateDetectionFilter, and in TopicRelevanceFilter retain only a stop_on_fail
annotation with its subclass-specific default, preserving the existing differing
defaults.
- Around line 18-24: Update InputColumn validation to enforce the relationship
between type and format: text columns must not specify format, while image and
pdf attachment columns must specify either url or base64. Add a cross-field
validator on InputColumn that rejects these invalid combinations during
configuration parsing and preserves the existing allowed values.
In `@backend/app/services/assessment/api/batch.py`:
- Around line 573-599: Introduce a dedicated enum for the poll outcomes
processing, completed, and failed, then update _poll_outcome’s return annotation
and all return values to use that enum. Replace the matching literal comparisons
in run_batch_stage with the enum members, keeping the existing control flow
unchanged.
- Around line 507-510: Remove the unused stage parameter from _row_subset, then
update its single call site to pass only bag, kind, and total while preserving
the existing subset-selection behavior.
- Around line 206-221: Update _prefilter_for_stage to return the concrete
pre-filter model type from blob.pre_filters instead of Any, using the existing
pre.topic_relevance and pre.duplicate_detection branches to keep the
stage-specific behavior intact. Then adjust build_pipeline to consume that
narrowed return type and propagate the same concrete annotation through its
parameter and return signatures, replacing any remaining Any-based pre-filter
typing with the specific model type already represented in the config objects.
In `@backend/app/services/assessment/api/results.py`:
- Around line 112-117: Update the result-building flow around build_rows to stop
reading input_schema from the execution bag, since that key is never written
there. Either obtain the schema from blob.assessment.params, as in the batch
flow, or remove the schema dependency when only len(rows) is used and document
that purpose with a brief comment.
In `@backend/app/services/llm/mappers.py`:
- Line 57: Update the annotations on _ensure_openai_strict_schema,
_strip_additional_properties, and _convert_json_schema_to_google to use
dict[str, Any] for both parameters and return values, preserving their existing
behavior and ensuring Any is available in the module imports.
- Around line 79-99: Update _strip_additional_properties to recursively traverse
schemas nested under anyOf, oneOf, allOf, and $defs, removing
additionalProperties from every branch while preserving the existing properties
and items handling. Reuse the traversal approach from
_ensure_openai_strict_schema or extract a shared walker with the functions’
existing node-specific transformations.
- Line 20: Update _convert_json_schema_to_google to wrap the
genai_transformers.t_schema call in compatibility handling and fall back to
normalized_schema if the private transformer is unavailable or changes behavior;
also tighten the google-genai upper dependency bound in pyproject.toml to a
tested compatible range.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b1304701-e579-4c92-abf3-aa5474327aef
📒 Files selected for processing (37)
backend/app/alembic/versions/076_refactor_assessment_tables.pybackend/app/api/docs/config/create.mdbackend/app/api/docs/config/delete.mdbackend/app/api/docs/config/get.mdbackend/app/api/main.pybackend/app/api/routes/assessment/api.pybackend/app/api/routes/assessment/assessments.pybackend/app/api/routes/assessment/runs.pybackend/app/api/routes/config/config.pybackend/app/celery/tasks/job_execution.pybackend/app/crud/assessment/__init__.pybackend/app/crud/assessment/api.pybackend/app/crud/assessment/batch.pybackend/app/crud/assessment/core.pybackend/app/crud/assessment/cron.pybackend/app/crud/assessment/processing.pybackend/app/crud/config/config.pybackend/app/crud/config/version.pybackend/app/crud/model_config.pybackend/app/models/assessment.pybackend/app/models/assessment/__init__.pybackend/app/models/assessment/assessment.pybackend/app/models/assessment/assessment_api.pybackend/app/models/config/assessment_blob.pybackend/app/models/config/config.pybackend/app/models/job.pybackend/app/models/llm/constants.pybackend/app/services/assessment/api/__init__.pybackend/app/services/assessment/api/batch.pybackend/app/services/assessment/api/callbacks.pybackend/app/services/assessment/api/results.pybackend/app/services/assessment/api/submission.pybackend/app/services/assessment/service.pybackend/app/services/llm/mappers.pybackend/app/tests/assessment/test_service.pydocs/wiki/domain-map.mddocs/wiki/modules/assessment.md
💤 Files with no reviewable changes (1)
- backend/app/models/assessment.py
Comment on lines
+81
to
+86
| * `pre_filters` (optional) — `topic_relevance` and/or `duplicate_detection`. Each runs its | ||
| own llm call, so it carries `provider` (default `openai`) + its own `params` | ||
| (TextLLMParams: `model`, `temperature`, …), plus its prompt (`topic_relevance.prompt` / | ||
| `duplicate_detection.content`, with an optional `knowledge_base_id` for duplicates) and | ||
| `stop_on_fail` (`true` = a failing verdict stops the chain and skips the assessment for | ||
| that row; `false` = the verdict is just recorded). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the differing stop_on_fail defaults.
TopicRelevanceFilter.stop_on_fail defaults to true and DuplicateDetectionFilter.stop_on_fail defaults to false (backend/app/models/config/assessment_blob.py lines 58-59 and 81-82). The text describes one option with no default, so a caller who omits the field cannot predict whether failing items are skipped.
State both defaults. Also consider replacing the internal class name TextLLMParams on line 83 with the field list, since this page is user-facing.
📝 Proposed wording
- `stop_on_fail` (`true` = a failing verdict stops the chain and skips the assessment for
- that row; `false` = the verdict is just recorded).
+ `stop_on_fail` (`true` = a failing verdict stops the chain and skips the assessment for
+ that row; `false` = the verdict is just recorded). It defaults to `true` for
+ `topic_relevance` and `false` for `duplicate_detection`.📝 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.
Suggested change
| * `pre_filters` (optional) — `topic_relevance` and/or `duplicate_detection`. Each runs its | |
| own llm call, so it carries `provider` (default `openai`) + its own `params` | |
| (TextLLMParams: `model`, `temperature`, …), plus its prompt (`topic_relevance.prompt` / | |
| `duplicate_detection.content`, with an optional `knowledge_base_id` for duplicates) and | |
| `stop_on_fail` (`true` = a failing verdict stops the chain and skips the assessment for | |
| that row; `false` = the verdict is just recorded). | |
| * `pre_filters` (optional) — `topic_relevance` and/or `duplicate_detection`. Each runs its | |
| own llm call, so it carries `provider` (default `openai`) + its own `params` | |
| (TextLLMParams: `model`, `temperature`, …), plus its prompt (`topic_relevance.prompt` / | |
| `duplicate_detection.content`, with an optional `knowledge_base_id` for duplicates) and | |
| `stop_on_fail` (`true` = a failing verdict stops the chain and skips the assessment for | |
| that row; `false` = the verdict is just recorded). It defaults to `true` for | |
| `topic_relevance` and `false` for `duplicate_detection`. |
🤖 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 `@backend/app/api/docs/config/create.md` around lines 81 - 86, Update the
pre_filters documentation to explicitly state that topic_relevance.stop_on_fail
defaults to true and duplicate_detection.stop_on_fail defaults to false. Replace
the internal TextLLMParams reference with its user-facing field list, including
model and temperature as applicable.
| "for assessment configs. Supported values: 'default', 'ASSESSMENT'." | ||
| ), | ||
| ), | ||
| ): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing return type annotations. Both changed endpoint signatures declare no return type, while list_configs in the same file declares -> APIResponse[list[ConfigPublic]]. The coding guidelines require a narrow type hint for every return value.
backend/app/api/routes/config/config.py#L100-L100: annotateget_configas-> APIResponse[ConfigPublic].backend/app/api/routes/config/config.py#L155-L155: annotatedelete_configas-> APIResponse[Message].
As per coding guidelines: "provide narrow type hints for every function parameter and return value; do not use -> Any as a substitute for a specific annotation".
📍 Affects 1 file
backend/app/api/routes/config/config.py#L100-L100(this comment)backend/app/api/routes/config/config.py#L155-L155
🤖 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 `@backend/app/api/routes/config/config.py` at line 100, In
backend/app/api/routes/config/config.py at lines 100-100 and 155-155, add narrow
return annotations to the get_config and delete_config endpoint signatures: use
APIResponse[ConfigPublic] for get_config and APIResponse[Message] for
delete_config, matching the existing list_configs annotation style.
Source: Coding guidelines
Comment on lines
+3
to
+27
| Assessments of datasets/inputs against ASSESSMENT-tagged configs. Three methods, chosen by input shape: **RESPONSE** (object input → `job` → llm_call/llm_chain), **BATCH** (list input → assessment_run → batch_job), **RUN** (legacy UI pipeline). No deep-dive doc yet. | ||
|
|
||
| All paths relative to `backend/app/`. | ||
|
|
||
| ## Routes | ||
| - `api/routes/assessment/` | ||
| - `api/routes/assessment/` — legacy UI (assessments, runs, datasets) | ||
| - `api/routes/assessment/api.py` — API-client route, mounted top-level at `/assessments` (**`POST /assessments` only** — no status/result poll endpoint; the result is delivered by webhook to the request's required `callback_url`); method inferred from input shape. BATCH wired; RESPONSE returns 501 (deferred) | ||
|
|
||
| ## Tables (SQLModel) | ||
| `models/assessment/` is a package, split by surface: `assessment.py` holds the shared DB tables + `AssessmentStatus`/`AssessmentMethod` enums + `AssessmentConfigRef` + the legacy RUN (UI) models; `assessment_api.py` holds the API-client request/response models. Both are re-exported from the package `__init__`, so `from app.models.assessment import X` resolves either. | ||
|
|
||
| | Table | Model | | ||
| |---|---| | ||
| | `assessment` (Assessment; FK → config, evaluation_dataset, batch_job, self-parent) | `models/assessment.py` | | ||
| | `assessment_run` (AssessmentRun) | `models/assessment.py` | | ||
| | `assessment` (Assessment; parent — `method`, data source; FK → job (RESPONSE), evaluation_dataset, org, project) | `models/assessment/assessment.py` | | ||
| | `assessment_run` (AssessmentRun; child — one config execution, BATCH/RUN; FK → assessment, config, batch_job) | `models/assessment/assessment.py` | | ||
|
|
||
| Config version (tag=ASSESSMENT, `models/config/assessment_blob.py`) owns system / pre-filters / params / schemas: `input_schema` (per-column spec `{type, strict, format}` for BATCH submissions — a `strict` column must be present in every submission; attachment columns are url-format only for batch) and `json_output_schema` (object-typed structured-output schema, omit for free text). Each pre-filter (`topic_relevance`, `duplicate_detection`) carries its own `provider` (default `openai`) + `params` (TextLLMParams: model, temperature, ...) and runs its own llm call. The request input carries the user-message. Strict input types `ResponseInput` (RESPONSE, `{query, attachments}`) / `BatchInput` (BATCH, `{query, data}` where `data` is a **list of submission rows** — each a flat column→string map, an attachment column's value being a url string — and `query` is a `{column}` template) both carry `query` and discriminate structurally on the other key (`data` ⇒ BATCH, else RESPONSE) with `extra=forbid` keeping them disjoint — no `mode` tag (`models/assessment/assessment_api.py`). Legacy RUN runtime lives in `assessment_run.execution` (`RunExecution`). | ||
|
|
||
| `assessment.id` is a **UUID** (like config/job/llm_call). Per-item result = `AssessmentResult {output: {assessment, pre_filter}, metadata, error}` where `output.assessment` = the LLM output parsed to an object when the config has a `json_output_schema`, else string (null for gated/failed rows), and `output.pre_filter` groups the two verdicts — `{topic_relevance, duplicate_detection}`, each `{verdict, reasoning}` or null — and is itself null when no pre-filter ran. Delivery is **webhook-only**: the `POST /assessments` ack is the flat `AssessmentSubmitResponse {assessment_id, status, message, inserted_at, updated_at}`, and the result is delivered solely by POSTing the `AssessmentCallback {assessment_id, status, data, request_metadata}` to the request's required `callback_url` on completion — where `data` is a single `AssessmentResult` (RESPONSE) or an `AssessmentBatchResult {total_items, counts, items}` (BATCH); `status` lives on the envelope only. Pre-filter `stop_on_fail` flag (`config/assessment_blob.py`) drives which filters hard-stop the chain on a failing verdict vs pass-through (record only). | ||
|
|
||
| ## Services / CRUD | ||
| - `services/assessment/` | ||
| - `crud/assessment/` — `batch.py` builds per-row prompts (`prompt_template` placeholder substitution `{column_name}`) | ||
| - `services/assessment/` — legacy RUN pipeline (service, stages, processing, batch, cron, tasks) | ||
| - `services/assessment/api/` — API-client pipeline: `submission.py` (submit), `batch.py` (staged provider batches — gate pre-filters → pass-through → assessment, over `core/batch`; `PREFILTER_VERDICT_SCHEMA`), `results.py` (builds `AssessmentBatchResult`), `callbacks.py` (webhook) | ||
| - `crud/assessment/api.py` — new API-client crud (method-based Assessment/AssessmentRun writes): `create_assessment`, `set_assessment_job`, `create_execution`, `set_execution_batch_job`, `update_status`, `list_executions` (no `get_assessment` — delivery is webhook-only, so there is no request-time fetch). Namespaced under `api` (`from app.crud.assessment import api`) to avoid colliding with the legacy `create_assessment`. | ||
| - `crud/assessment/{core,cron,processing,batch}.py` — legacy RUN pipeline crud. RUN runtime for the dropped columns (`stage`/`stage_status`/`pipeline`/`stage_batches`/`prefilter_total_*`/`object_store_url`) now lives in the `assessment_run.execution` bag via `core._read_exec`/`_write_exec`; `status` is the `AssessmentStatus` enum; the RUN input binding lives on the parent `assessment.input`. `batch.py` builds per-row prompts (`{column}` substitution from the parent `InputBinding.prompt`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct stale and conflicting module ownership text.
Remove “No deep-dive doc yet.” This page is now the deep-dive document.
Line 24 places processing, batch, and cron under services/assessment/. Line 27 correctly places those modules under crud/assessment/. Keep one ownership description so maintainers use the correct layer.
As per coding guidelines, **/*.{py,md} must use the repository's layered architecture.
🤖 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 `@docs/wiki/modules/assessment.md` around lines 3 - 27, Update the assessment
module documentation by removing “No deep-dive doc yet.” and correcting the
ownership descriptions so processing, batch, and cron are attributed only to
their CRUD layer under crud/assessment/, while services/assessment/ lists only
its actual service modules. Preserve the documented distinction between the
legacy RUN service pipeline and CRUD modules.
Source: Coding guidelines
…management in submission process
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/app/crud/config/config.py (1)
37-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExplain the validation purpose.
The comment restates the next call. State why validation occurs before persistence.
- # validate that the completion models in the config blob are valid + # Reject unavailable completion models before storing the first config version.🤖 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 `@backend/app/crud/config/config.py` around lines 37 - 38, Update the comment above validate_blob_completion_models to explain that completion models are validated before persistence to reject invalid configuration data before it is stored. Keep the comment focused on this validation purpose rather than restating the function call.Source: Coding guidelines
backend/app/tests/assessment/test_export.py (1)
33-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse narrow test-helper annotations.
AssessmentRun.statususesAssessmentStatus, but Lines 33 and 611 declarestatusasstr. Line 140 uses baredictforinput_data. Use the production enum and a parameterized dictionary type.
backend/app/tests/assessment/test_export.py#L33-L41: ChangestatustoAssessmentStatus.backend/app/tests/assessment/test_export.py#L135-L143: Changeinput_datatodict[str, str] | None.backend/app/tests/assessment/test_export.py#L611-L618: ChangestatustoAssessmentStatus.As per coding guidelines,
**/*.py: “Use Python 3.11+ and provide narrow type hints for every function parameter and return value; do not use-> Anyas a substitute for a specific annotation.”Proposed type updates
-def _run_ns(status: str = AssessmentStatus.PROCESSING) -> SimpleNamespace: +def _run_ns(status: AssessmentStatus = AssessmentStatus.PROCESSING) -> SimpleNamespace: - input_data: dict | None = None, + input_data: dict[str, str] | None = None, - def _make_run(self, *, status: str = AssessmentStatus.COMPLETED) -> MagicMock: + def _make_run( + self, *, status: AssessmentStatus = AssessmentStatus.COMPLETED + ) -> MagicMock:🤖 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 `@backend/app/tests/assessment/test_export.py` around lines 33 - 41, Use narrow annotations in backend/app/tests/assessment/test_export.py: in lines 33-41 and 611-618, change the _run_ns and corresponding helper status parameters from str to AssessmentStatus; in lines 135-143, change the input_data parameter from bare dict to dict[str, str] | None. Preserve the existing helper behavior and return annotations.Source: Coding guidelines
🤖 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 `@backend/app/services/assessment/service.py`:
- Around line 240-247: The retry flow around _build_retry_request must
explicitly reject assessments with more than four child runs before
start_assessment or request construction can produce an internal failure. Add a
400 response for runs exceeding the AssessmentRunCreate.configs limit, preserve
normal retry behavior for four or fewer runs, and add boundary coverage in the
assessment service tests.
---
Nitpick comments:
In `@backend/app/crud/config/config.py`:
- Around line 37-38: Update the comment above validate_blob_completion_models to
explain that completion models are validated before persistence to reject
invalid configuration data before it is stored. Keep the comment focused on this
validation purpose rather than restating the function call.
In `@backend/app/tests/assessment/test_export.py`:
- Around line 33-41: Use narrow annotations in
backend/app/tests/assessment/test_export.py: in lines 33-41 and 611-618, change
the _run_ns and corresponding helper status parameters from str to
AssessmentStatus; in lines 135-143, change the input_data parameter from bare
dict to dict[str, str] | None. Preserve the existing helper behavior and return
annotations.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 403dc4b4-4b3b-4476-8ec7-583837056b9c
📒 Files selected for processing (13)
backend/app/api/main.pybackend/app/api/routes/assessment/api.pybackend/app/api/routes/assessment/runs.pybackend/app/api/routes/config/config.pybackend/app/crud/config/config.pybackend/app/crud/model_config.pybackend/app/models/llm/constants.pybackend/app/services/assessment/api/results.pybackend/app/services/assessment/api/submission.pybackend/app/services/assessment/service.pybackend/app/services/assessment/utils/export.pybackend/app/services/llm/mappers.pybackend/app/tests/assessment/test_export.py
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/app/api/main.py
- backend/app/crud/model_config.py
- backend/app/services/assessment/api/results.py
- backend/app/services/llm/mappers.py
- backend/app/models/llm/constants.py
Comment on lines
240
to
247
| runs = get_assessment_runs_for_assessment( | ||
| session=session, assessment_id=assessment.id | ||
| ) | ||
| request = _build_retry_request( | ||
| experiment_name=assessment.experiment_name, | ||
| dataset_id=assessment.dataset_id, | ||
| input_binding=assessment.input, | ||
| runs=runs, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the retry reconstruction and its request-model limit.
rg -n -C 6 \
'def _build_retry_request|def retry_assessment|AssessmentRunCreate|configs:.*max_length' \
backend/app/services/assessment/service.py \
backend/app/models/assessment/assessment.py
# Check whether persistence or migrations constrain the number of child runs.
rg -n -C 5 \
'assessment_run|create_assessment_run|CheckConstraint|max_length|config_version' \
backend/app/models/assessment \
backend/app/alembic/versionsRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 50387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant service ranges =="
sed -n '45,90p' backend/app/services/assessment/service.py
sed -n '233,256p' backend/app/services/assessment/service.py
echo "== Routes handling retry_assessment =="
rg -n -C 8 'retry_assessment|retry.*assessment' backend/app -g '*.py'
echo "== Exceptions around routes/tests =="
rg -n -C 5 'HTTPException|pydantic|ValidationError|retry_assessment' backend/app tests backend/app/tests 2>/dev/null || true
echo "== Python probe: five configs vs max_length=4 =="
python3 - <<'PY'
import sys
sys.path.insert(0, 'backend')
try:
from app.models.assessment.assessment import AssessmentRunCreate, AssessmentConfigRef, InputBinding
except Exception as exc:
print(f"IMPORT_ERROR: {type(exc).__name__}: {exc}")
raise SystemExit(0)
valid_request = AssessmentRunCreate(
experiment_name="x",
dataset_id=1,
input_binding=InputBinding(type="RUN", experiment_ids=[], batch_job_ids=[]),
configs=[
AssessmentConfigRef(id="00000000-0000-0000-0000-000000000001", version=1),
AssessmentConfigRef(id="00000000-0000-0000-0000-000000000002", version=1),
AssessmentConfigRef(id="00000000-0000-0000-0000-000000000003", version=1),
AssessmentConfigRef(id="00000000-0000-0000-0000-000000000004", version=1),
],
)
print("VALID_REQUEST_CONFIGS=", len(valid_request.configs))
try:
AssessmentRunCreate(
experiment_name="x",
dataset_id=1,
input_binding=InputBinding(type="RUN", experiment_ids=[], batch_job_ids=[]),
configs=[
AssessmentConfigRef(id=f"00000000-0000-0000-0000-00000000000{i}", version=1) for i in range(1, 6)
],
)
print("VALID_FIVE_CONFIGS=unexpected")
except Exception as exc:
print("FIVE_CONFIGS_ERROR=", type(exc).__name__)
print("FIVE_CONFIGS_MESSAGE=", str(exc).replace("\n", " ")[:500])
PYRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 50388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Focused service lines =="
sed -n '45,90p' backend/app/services/assessment/service.py
sed -n '210,256p' backend/app/services/assessment/service.py
echo "== Focused model lines =="
sed -n '216,245p' backend/app/models/assessment/assessment.py
sed -n '330,342p' backend/app/models/assessment/assessment.py
echo "== Focused route search results only =="
rg -n 'retry_assessment|retry' backend/app/api -g '*.py' | head -120
rg -n 'retry_assessment|retry' backend/app/tests backend/tests 2>/ dev/null | head -120 || true
echo "== Exception handlers and validation status =="
rg -n -C 4 'ValidationError|status_code=422|HTTPException|pydantic' backend/app/core backend/app/api -g '*.py' | head -160
echo "== Python read-only probe for ValidationError type =="
python3 - <<'PY'
import sys
try:
import pydantic.version
print("pydantic_version=", pydantic.version.VERSION)
except Exception as exc:
print("PYDANTIC_IMPORT_ERROR=", type(exc).__name__, str(exc))
raise SystemExit(0)
sys.path.insert(0, 'backend')
from app.models.assessment.assessment import AssessmentRunCreate, AssessmentConfigRef, InputBinding
from pydantic import ValidationError
try:
AssessmentRunCreate(
experiment_name="x",
dataset_id=1,
input_binding=InputBinding(type="RUN", experiment_ids=[], batch_job_ids=[]),
configs=[AssessmentConfigRef(id=f"00000000-0000-0000-0000-00000000000{i}", version=1) for i in range(1, 6)],
)
print("ACCEPTED_FIVE_CONFIGS=unexpected")
except ValidationError:
print("ACCEPTED_FIVE_CONFIGS=unlikely")
except Exception as exc:
print("ACCEPTED_FIVE_CONFIGS=unusual_exception", type(exc).__name__)
try:
AssessmentRunCreate(
experiment_name="x",
dataset_id=1,
input_binding=InputBinding(type="RUN", experiment_ids=[], batch_job_ids=[]),
configs=[AssessmentConfigRef(id="00000000-0000-0000-0000-000000000001", version=1) for _ in range(4)],
)
print("ACCEPTED_FOUR_CONFIGS=expected")
except Exception as exc:
print("ACCEPTED_FOUR_CONFIGS=unexpected", type(exc).__name__, str(exc)[:200])
PYRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 15401
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Assessment route wrapper =="
sed -n '60,90p' backend/app/api/routes/assessment/assessments.py
echo "== Core exception handler full =="
sed -n '1,130p' backend/app/core/exception_handlers.py
echo "== Deterministic Pydantic max_length probe without repo model dependency =="
python3 - <<'PY'
from typing import Annotated
try:
from pydantic import BaseModel, Field, ValidationError
except Exception as exc:
print("PYDANTIC_NOT_INSTALLED")
raise SystemExit(0)
class Entry(BaseModel):
value: int
class Request(BaseModel):
configs: list[Entry] = Field(min_length=1, max_length=4)
try:
Request(configs=[Entry(value=i) for i in range(5)])
print("ACCEPTED_FIVE=unexpected")
except ValidationError as exc:
errors = exc.errors()
print("REJECTED_FIVE=True")
print("ERROR_COUNT=", len(errors))
print("FIRST_ERROR_LOC=", errors[0].get("loc"))
print("FIRST_ERROR_TYPE=", errors[0].get("type"))
try:
Request(configs=[Entry(value=i) for i in range(4)])
print("ACCEPTED_FOUR=True")
except Exception as exc:
print("ACCEPTED_FOUR=", type(exc).__name__, str(exc)[:200])
PYRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 5451
Handle retry requests with more than four child runs explicitly.
_build_retry_request passes every child run into AssessmentRunCreate.configs, but AssessmentRunCreate limits configs to four entries. An assessment with five or more runs fails during request construction: raise a 400 before start_assessment, or change the intended max before requiring callers to catch retry failures as a 500. Add coverage in backend/app/tests/services/assessment for the boundary.
🤖 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 `@backend/app/services/assessment/service.py` around lines 240 - 247, The retry
flow around _build_retry_request must explicitly reject assessments with more
than four child runs before start_assessment or request construction can produce
an internal failure. Add a 400 response for runs exceeding the
AssessmentRunCreate.configs limit, preserve normal retry behavior for four or
fewer runs, and add boundary coverage in the assessment service tests.
- Updated tests to reflect changes in how assessment run state is managed, moving runtime state (stage, stage_status, etc.) to an execution bag. - Adjusted assertions in tests to access state from the execution bag instead of direct attributes. - Changed instruction handling in assessment batch processing to derive from configuration rather than request parameters. - Updated assessment run creation and retry logic to use new input binding structure. - Ensured consistency in status naming conventions across tests and models (e.g., "processing" to "PROCESSING"). - Added fixtures to stub flag modification for better test isolation.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@backend/app/services/assessment/tasks.py`:
- Around line 170-186: Use AssessmentStatus enum values for all affected status
assignments and assertions: in backend/app/services/assessment/tasks.py lines
170-186, assign AssessmentStatus.PROCESSING; in
backend/app/services/assessment/stages.py lines 195-203, assign
AssessmentStatus.COMPLETED; and in backend/app/tests/assessment/test_pipeline.py
lines 96-109, assert AssessmentStatus.COMPLETED instead of lowercase string
literals.
In `@backend/app/tests/assessment/test_prefilter_batching.py`:
- Around line 31-49: Apply narrow type annotations to all affected test helpers
and methods: in backend/app/tests/assessment/test_prefilter_batching.py lines
31-49, annotate _session_cm and _run, including **kw and return types; in
backend/app/tests/assessment/test_processing.py lines 20-26 and
backend/app/tests/assessment/test_cron.py lines 15-46, annotate
_stub_flag_modified with its iterator return type, and in test_cron.py replace
dict | None with a parameterized execution mapping; in
backend/app/tests/assessment/test_crud.py lines 299-351, annotate each modified
test method with typed self and -> None.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d0fbbe4a-68f5-4349-a532-6716455086b7
📒 Files selected for processing (14)
backend/app/api/docs/config/create.mdbackend/app/models/config/assessment_blob.pybackend/app/services/assessment/service.pybackend/app/services/assessment/stages.pybackend/app/services/assessment/tasks.pybackend/app/tests/assessment/test_batch.pybackend/app/tests/assessment/test_cron.pybackend/app/tests/assessment/test_crud.pybackend/app/tests/assessment/test_pipeline.pybackend/app/tests/assessment/test_prefilter_batching.pybackend/app/tests/assessment/test_processing.pybackend/app/tests/assessment/test_routes.pybackend/app/tests/assessment/test_service.pybackend/app/tests/assessment/test_tasks_failure_guard.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/api/docs/config/create.md
Comment on lines
31
to
49
| @contextmanager | ||
| def _session_cm(session): | ||
| yield session | ||
|
|
||
|
|
||
| def _run(**kw): | ||
| execution = {key: kw.pop(key) for key in _EXEC_BAG_KEYS if key in kw} | ||
| kw.pop("input", None) # run input now lives on the parent assessment | ||
| base = { | ||
| "id": 5, | ||
| "assessment_id": 9, | ||
| "input": { | ||
| "prefilter_config": {"topic_relevance": {"columns": ["a"], "prompt": "p"}} | ||
| }, | ||
| "config_id": "c", | ||
| "config_version": 1, | ||
| "pipeline": None, | ||
| "stage": None, | ||
| "stage_status": None, | ||
| "status": "pending", | ||
| "stage_batches": None, | ||
| "total_items": 0, | ||
| } | ||
| base.update(kw) | ||
| base["execution"] = execution | ||
| return SimpleNamespace(**base) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add narrow type annotations to the changed test helpers and methods.
backend/app/tests/assessment/test_prefilter_batching.py#L31-L49: annotate_session_cmand_run, including**kwand the return types.backend/app/tests/assessment/test_processing.py#L20-L26: annotate_stub_flag_modifiedwith its iterator return type.backend/app/tests/assessment/test_cron.py#L15-L46: annotate_stub_flag_modified; replacedict | Nonewith a parameterized execution mapping.backend/app/tests/assessment/test_crud.py#L299-L351: annotate the modified test methods with typedselfand-> None.
As per coding guidelines, **/*.py requires narrow type hints for every function parameter and return value.
📍 Affects 4 files
backend/app/tests/assessment/test_prefilter_batching.py#L31-L49(this comment)backend/app/tests/assessment/test_processing.py#L20-L26backend/app/tests/assessment/test_cron.py#L15-L46backend/app/tests/assessment/test_crud.py#L299-L351
🤖 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 `@backend/app/tests/assessment/test_prefilter_batching.py` around lines 31 -
49, Apply narrow type annotations to all affected test helpers and methods: in
backend/app/tests/assessment/test_prefilter_batching.py lines 31-49, annotate
_session_cm and _run, including **kw and return types; in
backend/app/tests/assessment/test_processing.py lines 20-26 and
backend/app/tests/assessment/test_cron.py lines 15-46, annotate
_stub_flag_modified with its iterator return type, and in test_cron.py replace
dict | None with a parameterized execution mapping; in
backend/app/tests/assessment/test_crud.py lines 299-351, annotate each modified
test method with typed self and -> None.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/app/models/config/assessment_blob.py (1)
65-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse named constants for pre-filter defaults.
"high"and"auto"are persisted configuration values. Define them as constants or enum members and reuse them here.As per coding guidelines: “Do not use magic values; extract repeated literals into constants, enums, or settings.”
🤖 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 `@backend/app/models/config/assessment_blob.py` around lines 65 - 67, Replace the persisted pre-filter default literals in the model-default branch with named constants or enum members for the effort and summary values, defining them in the appropriate configuration module and reusing them in this setdefault logic.Source: Coding guidelines
🤖 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 `@backend/app/api/docs/assessment/create.md`:
- Around line 78-79: Update the acknowledgement response example in the
assessment creation documentation to match the runtime response from the
submission service: use status PROCESSING and message "Your assessment is being
processed" instead of the current PENDING status and submission message.
---
Nitpick comments:
In `@backend/app/models/config/assessment_blob.py`:
- Around line 65-67: Replace the persisted pre-filter default literals in the
model-default branch with named constants or enum members for the effort and
summary values, defining them in the appropriate configuration module and
reusing them in this setdefault logic.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e667c38b-6361-414a-b618-2124d9ee9668
📒 Files selected for processing (6)
backend/app/api/docs/assessment/create.mdbackend/app/api/routes/assessment/api.pybackend/app/models/config/assessment_blob.pybackend/app/services/assessment/stages.pybackend/app/services/assessment/tasks.pybackend/app/tests/assessment/test_pipeline.py
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/app/tests/assessment/test_pipeline.py
- backend/app/services/assessment/stages.py
- backend/app/api/routes/assessment/api.py
- backend/app/services/assessment/tasks.py
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
vprashrex
changed the title
…rom version endpoints
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/crud/config/version.py (1)
30-36: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the constructor return annotation.
Add
-> NonetoConfigVersionCrud.__init__.Proposed fix
def __init__( self, session: Session, config_id: UUID, project_id: int, tag: ConfigTag | None = None, - ): + ) -> None:As per coding guidelines,
**/*.pyrequires narrow type hints for every function parameter and return value.🤖 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 `@backend/app/crud/config/version.py` around lines 30 - 36, Add the explicit -> None return annotation to ConfigVersionCrud.__init__, preserving its existing parameter annotations and implementation.Source: Coding guidelines
🤖 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 `@backend/app/crud/config/version.py`:
- Around line 49-52: Update the HTTPException in the config lookup flow to use
the named status.HTTP_404_NOT_FOUND constant instead of the numeric 404 value,
ensuring the required status module is imported and preserving the existing
not-found behavior.
---
Outside diff comments:
In `@backend/app/crud/config/version.py`:
- Around line 30-36: Add the explicit -> None return annotation to
ConfigVersionCrud.__init__, preserving its existing parameter annotations and
implementation.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d253aed-cc44-4188-9d0c-e6520ae5fa6d
📒 Files selected for processing (9)
backend/app/api/docs/config/create.mdbackend/app/api/docs/config/create_version.mdbackend/app/api/docs/config/delete.mdbackend/app/api/docs/config/get.mdbackend/app/api/docs/config/get_version.mdbackend/app/api/docs/config/list_versions.mdbackend/app/api/routes/config/version.pybackend/app/crud/config/version.pybackend/app/tests/crud/config/test_version.py
💤 Files with no reviewable changes (4)
- backend/app/api/docs/config/list_versions.md
- backend/app/api/docs/config/get_version.md
- backend/app/api/docs/config/create_version.md
- backend/app/api/routes/config/version.py
Comment on lines
+49
to
+52
| raise HTTPException( | ||
| status_code=404, | ||
| detail=f"config with id '{config_id}' not found", | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the numeric HTTP status with a named constant.
Use status.HTTP_404_NOT_FOUND instead of 404. This removes a repeated magic value and makes the response status explicit.
As per coding guidelines, **/*.py says: “Do not use magic values; extract repeated literals into constants, enums, or settings.”
🤖 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 `@backend/app/crud/config/version.py` around lines 49 - 52, Update the
HTTPException in the config lookup flow to use the named
status.HTTP_404_NOT_FOUND constant instead of the numeric 404 value, ensuring
the required status module is imported and preserving the existing not-found
behavior.
Source: Coding guidelines
vprashrex
changed the title
vprashrex
commented
Aug 9, 2026
| *, | ||
| session: Session, | ||
| method: AssessmentMethod, | ||
| input: dict[str, Any], |
Collaborator
Author
There was a problem hiding this comment.
Any is intentional here. The JSONB column is polymorphic by method, and shape validation is enforced at the edges. dict[str, Any] at the persistence layer is the correct abstraction, not a type-safety gap.
Adding strict typing here would be over-engineering since validation already happens downstream via batch_input.model_dump(mode="json").
…e type annotations
- Implement comprehensive tests for the assessment API-client CRUD operations in `test_api_crud.py`, covering creation, execution, state saving, and status updates. - Introduce tests for the BATCH API-client submission entrypoint in `test_api_submission.py`, validating assessment creation, error handling for strict schema violations, unsupported providers, and dispatch failures. - Ensure proper validation and response handling for various input scenarios, including base64 attachments and response inputs.
…d function definitions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Closes #1098
Summary
Before: Config + versioning module only handled
DEFAULTLLM configs. Assessment runs had no first-class config, so no way to store or version them.Now:
tag:ConfigCreate.config_blobacceptsConfigBlob(tag: DEFAULT) orAssessmentConfigBlob(tag: ASSESSMENT); a validator rejects a blob whose shape does not match its tag.AssessmentConfigBlob:assessment(required grading completion) + optionalpre_filters(topic_relevance,duplicate_detection), each running its own LLM call or Batch. Pre-filterparamsdefault togpt-5.6-luna.api/docs/config/create.md.Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Assessment SRD Doc:
https://docs.google.com/document/d/1G_zzUxtW_5wu-3unKwiU99kgkvgDwSht4dcblRsN6SE/edit?usp=sharing