feat(flow): add ranking context assets#604
Conversation
Code ReviewOverviewThis PR adds an optional Issues1. Mustache templates not updated (required by CLAUDE.md)
No mustache template changes are included in this PR. On the next OpenAPI codegen run the changes to 2. Documentation not updated (required by CLAUDE.md)CLAUDE.md states:
3. Pyright not run (required by CLAUDE.md)The PR description explicitly notes pyright was not run. CLAUDE.md requires running Minor observationsValidation placement — The Upload order —
No test coverage — There are no tests for the new parameter path. Given the PR description also notes the code was not run locally, test coverage is especially important here. SummaryThe core logic (validation, upload, model field wiring) is correct and follows existing patterns. Before merging:
|
Co-Authored-By: luca <luca@rapidata.ai>
Regenerated against the merged backend flow contract (api.rabbitdata.ch). Adds contextAsset to the flow item query/get output models that the manual edit had missed, and refreshes the merged schema specs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Co-Authored-By: luca <luca@rapidata.ai>
197b913 to
9c3c919
Compare
PR Review: feat(flow): add ranking context assetsOverall this is a clean addition that follows existing patterns well. A few issues worth addressing before merging. 🔴 CriticalMustache templates not updated
The three auto-generated model files were modified directly:
Without corresponding 🟡 MediumDocumentation not updated (
flow_item = flow.create_new_flow_batch(
datapoints=[...],
context_assets=["https://example.com/reference.jpg"], # not documented
)Pyright not run
Missing All sibling optional fields carry 🔵 MinorNo guard against passing both A caller can currently supply both simultaneously with no feedback. The docstring doesn't clarify the relationship between the two. At minimum, the docstring should say whether they are independent or mutually exclusive. Inconsistent Existing models with a if self.context_asset is None and "context_asset" in self.model_fields_set:
_dict['contextAsset'] = NoneThe three new models omit this. If
# rapidata_flow.py:96
context_asset_input = (
AssetUploader(self._openapi_service).upload_and_map_asset(context_assets)
if context_assets
else None
)
Dataset left orphaned on context-asset upload failure If the dataset and datapoints upload succeed but ✅ Looks Good
|
Summary
Validation
🔗 Session: https://session-f0bf5a67.poseidon.rapidata.internal/