Skip to content

data_resources - #24

Merged
alain-sv merged 11 commits into
developfrom
data_resources
Apr 14, 2026
Merged

data_resources#24
alain-sv merged 11 commits into
developfrom
data_resources

Conversation

@alain-sv

Copy link
Copy Markdown
Contributor

Summary- Add DataResource and DataResourceField models- metadata field to AbstractJob and CaseAbstractModel- Introduce FieldType enum, fix computed_field behavior, and clarify optional callbacks- Add data_resources field and registration_info support to agent- Generate CRUD routes for DataResource with factory functions for safe capture and unique operationIds- Remove duplicate entries from __all- Update CHANGELOG and version for DataResource release add test results section and release header for0.13.3

…lts" section to CHANGELOG.md showing thelatest test summary (492 passed,0,0 failed, ~70).

Insert a new release header for0.13.3 dated202604-14 preservethe existing "Added" entries describing CaseNodeUpdate.upsert /
Case.patch and human-answer casestep_index.

This documents the test health and records the new patch release sousers and maintainers can see the verification status alongside thefeature notes.
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Add DataResource CRUD framework with auto-generated routes and metadata support

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add DataResource and DataResourceField models for agent-owned CRUD endpoints
• Introduce FieldType and Editable enums for field validation and form control
• Add metadata field to AbstractJob and CaseAbstractModel for custom context
• Auto-generate FastAPI CRUD routes for declared DataResources with bulk import support
• Include data_resources in Agent.registration_info for Studio discovery
Diagram
flowchart LR
  A["Agent declares<br/>DataResources"] -->|includes| B["DataResource<br/>with callbacks"]
  B -->|defines| C["DataResourceField<br/>with FieldType"]
  B -->|triggers| D["Auto-generated<br/>CRUD routes"]
  D -->|secured by| E["API key auth"]
  A -->|exposes via| F["registration_info"]
  G["Job/Case<br/>metadata"] -->|flows to| F
Loading

Grey Divider

File Changes

1. src/supervaizer/data_resource.py ✨ Enhancement +154/-0

New DataResource and field schema models

src/supervaizer/data_resource.py


2. src/supervaizer/data_routes.py ✨ Enhancement +166/-0

FastAPI CRUD route generation for DataResources

src/supervaizer/data_routes.py


3. src/supervaizer/agent.py ✨ Enhancement +9/-0

Add data_resources field and registration support

src/supervaizer/agent.py


View more (9)
4. src/supervaizer/job.py ✨ Enhancement +5/-1

Add metadata field to AbstractJob model

src/supervaizer/job.py


5. src/supervaizer/case.py ✨ Enhancement +5/-1

Add metadata field to CaseAbstractModel

src/supervaizer/case.py


6. src/supervaizer/routes.py ✨ Enhancement +3/-0

Integrate DataResource routes into agent routing

src/supervaizer/routes.py


7. src/supervaizer/__init__.py ✨ Enhancement +5/-3

Export DataResource classes and remove duplicates

src/supervaizer/init.py


8. tests/test_data_resource.py 🧪 Tests +109/-0

Comprehensive tests for DataResource validation

tests/test_data_resource.py


9. tests/test_agent.py 🧪 Tests +41/-0

Test agent data_resources field and registration

tests/test_agent.py


10. tests/test_job.py 🧪 Tests +25/-0

Test job metadata field and registration

tests/test_job.py


11. tests/test_case.py 🧪 Tests +39/-0

Test case metadata field and registration

tests/test_case.py


12. docs/CHANGELOG.md 📝 Documentation +31/-0

Document DataResource feature and test results

docs/CHANGELOG.md


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Apr 14, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)
🐞\ ≡ Correctness (1)

Grey Divider


Action required

1. test_data_resource lacks return types📘
Description
New pytest tests in tests/test_data_resource.py define functions without explicit return type
annotations (e.g., missing -> None). This violates the project requirement for type-hinted,
mypy-clean Python code and can fail mypy if annotations are enforced.
Code

tests/test_data_resource.py[R11-27]

+def test_field_defaults():
+    f = DataResourceField(name="email")
+    assert f.field_type == FieldType.STRING
+    assert f.editable == Editable.ALWAYS
+    assert f.visible_on == ["list", "detail", "create", "edit"]
+    assert f.required is False
+
+
+def test_field_display_label_defaults_to_name_title():
+    f = DataResourceField(name="first_name")
+    assert f.display_label == "First Name"
+
+
+def test_field_display_label_custom():
+    f = DataResourceField(name="first_name", label="Given Name")
+    assert f.display_label == "Given Name"
+
Evidence
PR Compliance ID 116967 requires explicit type hints (including return types) for all new/modified
Python functions. The newly added test functions in tests/test_data_resource.py omit return type
annotations.

Rule 116967: Enforce type hints and mypy-clean Python code
tests/test_data_resource.py[11-27]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New tests in `tests/test_data_resource.py` are missing explicit return type annotations, which violates the repo's type-hints/mypy-clean requirement.
## Issue Context
The compliance rule requires explicit type hints (including return types) for all functions in new/modified Python files.
## Fix Focus Areas
- tests/test_data_resource.py[11-109]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. on_update return type wrong📘
Description
DataResource.on_update is annotated as always returning dict[str, Any], but the new CRUD route
handler treats a None return as a not-found condition. This type mismatch is likely to cause mypy
errors and violates the type-hints/mypy-clean requirement.
Code

src/supervaizer/data_resource.py[R102-106]

+    on_list: Callable[[], list[dict[str, Any]]] | None = Field(default=None, exclude=True)
+    on_get: Callable[[str], dict[str, Any] | None] | None = Field(default=None, exclude=True)
+    on_create: Callable[[dict[str, Any]], dict[str, Any]] | None = Field(default=None, exclude=True)
+    on_update: Callable[[str, dict[str, Any]], dict[str, Any]] | None = Field(default=None, exclude=True)
+    on_delete: Callable[[str], bool] | None = Field(default=None, exclude=True)
Evidence
PR Compliance ID 116967 requires mypy-clean code with correct type hints. on_update is typed as
returning a non-optional dict, while the new route code checks if result is None, indicating the
callback may return None and making the annotation inconsistent.

Rule 116967: Enforce type hints and mypy-clean Python code
src/supervaizer/data_resource.py[102-106]
src/supervaizer/data_routes.py[142-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DataResource.on_update` is annotated as returning `dict[str, Any]` but the update route treats `None` as a valid "not found" sentinel. This mismatch can break mypy/type-checking.
## Issue Context
The update handler raises 404 when the callback returns `None`, implying `on_update` should be able to return `None`.
## Fix Focus Areas
- src/supervaizer/data_resource.py[102-106]
- src/supervaizer/data_routes.py[142-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. result slice not Black📘
Description
The slice expression result[skip: skip + limit] is not Black-formatted and will be reformatted by
black --check. This violates the requirement to enforce Black formatting with the project
configuration.
Code

src/supervaizer/data_routes.py[115]

+        return result[skip: skip + limit]
Evidence
PR Compliance ID 116966 requires Black formatting to pass. The added slice formatting in
data_routes.py is inconsistent with Black’s slice spacing and is likely to fail black --check.

Rule 116966: Enforce Black formatting with project configuration
src/supervaizer/data_routes.py[113-116]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/supervaizer/data_routes.py` contains a slice expression that is not formatted according to Black, which can cause CI formatting checks to fail.
## Issue Context
Black enforces consistent spacing rules for slice expressions.
## Fix Focus Areas
- src/supervaizer/data_routes.py[113-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (4)
4. OperationId collisions🐞
Description
DataResource CRUD routes set OpenAPI operation_id values using only resource.name (e.g.
"contacts_list"), which will collide when multiple agents (or multiple resources) share the same
resource name in the same FastAPI app and can break OpenAPI/client generation.
Code

src/supervaizer/data_routes.py[R47-55]

+    if resource.on_list is not None:
+        router.add_api_route(
+            f"{prefix}/",
+            _make_list_handler(resource, prefix),
+            methods=["GET"],
+            dependencies=[Security(server.verify_api_key)],
+            summary=f"List {resource.display_name_resolved}",
+            operation_id=f"{resource.name}_list",
+        )
Evidence
All agents are mounted into one app router, so two agents exposing the same resource name will
register endpoints with identical operation_id values. The codebase already treats unique operation
IDs as important for custom routes by including agent.slug in the generated identifier.

src/supervaizer/data_routes.py[43-105]
src/supervaizer/routes.py[383-393]
src/supervaizer/routes.py[928-951]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/supervaizer/data_routes.py` sets `operation_id=f"{resource.name}_..."` for every DataResource route. Because `create_agents_routes()` mounts routers for *all* agents into the same FastAPI app, operation IDs will collide across agents that expose the same resource name (and can also collide if two resources share a name).
### Issue Context
The code already attempts to keep custom routes uniquely identified by including `agent.slug` (see custom routes). DataResource routes should follow the same uniqueness rule to avoid OpenAPI schema/client-gen issues.
### Fix Focus Areas
- src/supervaizer/data_routes.py[35-105]
- src/supervaizer/routes.py[383-393]
- src/supervaizer/routes.py[928-951]
### Suggested fix
- Pass `agent.slug` into `_add_resource_routes(...)` (or build routes inside `create_agent_data_routes` with access to `agent`).
- Set `operation_id` to include the agent, e.g. `f"{agent.slug}_{resource.name}_list"`.
- (Optional) also set the `name=` parameter consistently for the route to match the operation_id and avoid ambiguity.
- (Optional) validate uniqueness of `resource.name` within `agent.data_resources` during agent initialization/registration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Metadata breaks event JSON🐞
Description
Job and Case now include metadata: dict[str, Any] verbatim in registration_info, and those dicts are
used as Event.details sent via httpx json=...; non-JSON values in metadata (e.g. datetime, type,
custom objects) will raise at send time.
Code

src/supervaizer/job.py[R350-354]

         "finished_at": self.finished_at.isoformat() if self.finished_at else "",
         "created_at": self.created_at.isoformat() if self.created_at else "",
         "case_ids": self.case_ids,
+            "metadata": self.metadata,
     }
Evidence
Event constructors use job.registration_info / case.registration_info as the details payload,
and account_service.send_event posts using json=payload which relies on Python JSON encoding.
Because metadata is typed as Any and not serialized/validated, it can easily contain values that
are not JSON encodable, crashing event emission.

src/supervaizer/job.py[244-260]
src/supervaizer/job.py[338-354]
src/supervaizer/case.py[206-219]
src/supervaizer/case.py[352-366]
src/supervaizer/event.py[117-129]
src/supervaizer/event.py[155-165]
src/supervaizer/account_service.py[67-81]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Job.registration_info` and `Case.registration_info` now include `metadata` as a raw `dict[str, Any]`. Those dicts are embedded into `Event.payload`, and `send_event()` sends them via `httpx.post(..., json=payload)`. If an agent puts non-JSON-encodable values into metadata (datetime, type, custom objects), JSON encoding fails and event emission crashes.
### Issue Context
The SDK already has `SvBaseModel.serialize_value(...)` which converts some common non-JSON types (e.g. datetime/type). Using it here would make metadata safer and more consistent.
### Fix Focus Areas
- src/supervaizer/job.py[338-354]
- src/supervaizer/case.py[352-366]
- src/supervaizer/account_service.py[67-81]
- src/supervaizer/common.py[36-69]
### Suggested fix
Choose one (or combine):
1) **Serialize on output**: in `registration_info`, set `"metadata": SvBaseModel.serialize_value(self.metadata)`.
2) **Validate on assignment**: add a validator for `metadata` that attempts `json.dumps(...)` (or `orjson.dumps(...)`) and raises a clear error if not JSON-safe.
3) **Constrain the type**: use a JSON-safe type (e.g. `JsonValue`) if you want to enforce at model level.
Add a unit test that sets metadata with a datetime and verifies event payload serialization succeeds.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Update method contradicts CHANGELOG🐞
Description
The CHANGELOG states DataResource CRUD routes include PATCH for updates, but the generated update
route uses PUT; clients following the documented contract will receive 405 Method Not Allowed.
Code

src/supervaizer/data_routes.py[R77-85]

+    if resource.on_update is not None and not resource.read_only:
+        router.add_api_route(
+            f"{prefix}/{{item_id}}",
+            _make_update_handler(resource, prefix),
+            methods=["PUT"],
+            dependencies=[Security(server.verify_api_key)],
+            summary=f"Update {resource.display_name_resolved}",
+            operation_id=f"{resource.name}_update",
+        )
Evidence
Repo documentation explicitly describes PATCH for CRUD updates, while the implementation registers
the update handler with methods=["PUT"]. This is an externally visible API contract mismatch.

docs/CHANGELOG.md[36-38]
src/supervaizer/data_routes.py[77-85]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The repo CHANGELOG documents DataResource update as PATCH, but the implementation registers update as PUT. This creates a breaking contract mismatch for Studio/clients.
### Issue Context
If updates are partial (typical for forms), PATCH is the more appropriate method. If the intent is full replacement, update the CHANGELOG to match PUT.
### Fix Focus Areas
- src/supervaizer/data_routes.py[77-85]
- docs/CHANGELOG.md[36-38]
### Suggested fix
- Prefer: change `methods=["PUT"]` to `methods=["PATCH"]`.
- Or: register both PUT and PATCH to the same handler if you want to be tolerant.
- Ensure the OpenAPI operation_id/name remain unique after this change.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. CSV import not supported 🐞
Description
The CHANGELOG claims the bulk import route accepts CSV or JSON, but the import handler only accepts
a JSON body (list[dict]); CSV uploads will fail validation.
Code

src/supervaizer/data_routes.py[R162-166]

+def _make_import_handler(r: DataResource, prefix: str) -> Any:
+    async def _handler(records: list[dict[str, Any]] = Body(...)) -> dict[str, Any]:
+        log.info(f"📥 POST {prefix}/import/ [DataResource import: {r.name}]")
+        return r.on_import(records)  # type: ignore[misc]
+    return _handler
Evidence
The documented contract includes CSV support, but the route handler signature only supports JSON and
there is no code path for parsing CSV content (e.g. UploadFile/multipart or text/csv parsing).

docs/CHANGELOG.md[38-39]
src/supervaizer/data_routes.py[97-105]
src/supervaizer/data_routes.py[162-166]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The bulk import route is documented as accepting CSV or JSON, but the handler only accepts JSON (`records: list[dict[str, Any]] = Body(...)`). CSV content will not be accepted.
### Issue Context
This is an API contract issue. Either implement CSV parsing or update the documentation to remove CSV claims.
### Fix Focus Areas
- src/supervaizer/data_routes.py[97-105]
- src/supervaizer/data_routes.py[162-166]
- docs/CHANGELOG.md[38-39]
### Suggested fix
Option A (implement CSV):
- Accept `UploadFile` (multipart/form-data) or raw `text/csv` body.
- Parse CSV into `list[dict[str, Any]]` (header row -> keys), then call `on_import(records)`.
- Keep existing JSON support.
Option B (docs-only):
- Update CHANGELOG and any docs to state JSON-only import until CSV is implemented.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

8. Unsafe resource name🐞
Description
DataResource.name is interpolated directly into route paths and operation_ids without validation, so
names containing characters like '/', '{', or '}' can create malformed/unintended routes or invalid
OpenAPI identifiers.
Code

src/supervaizer/data_routes.py[R43-46]

+def _add_resource_routes(router: APIRouter, resource: DataResource, server: "Server") -> None:
+    """Register all declared operation routes for one DataResource."""
+    prefix = f"/data/{resource.name}"
+
Evidence
The router prefix is built from resource.name with no sanitization, and the DataResource model
only documents the name as "URL-safe" without enforcing it via validation. This can cause
startup-time routing errors or broken OpenAPI output depending on the string used.

src/supervaizer/data_routes.py[43-55]
src/supervaizer/data_resource.py[93-97]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DataResource.name` is used to form URL paths (`/data/{resource.name}`) and OpenAPI IDs. Without validation, a name containing `/` or `{}` can create malformed routes or invalid OpenAPI identifiers.
### Issue Context
This is most likely a configuration error by an agent author, but because it impacts server startup and docs generation, it’s worth enforcing early.
### Fix Focus Areas
- src/supervaizer/data_resource.py[93-127]
- src/supervaizer/data_routes.py[43-55]
### Suggested fix
- Add a Pydantic validator on `DataResource.name` to enforce a conservative pattern (e.g. `^[a-z0-9][a-z0-9_-]*$`).
- Consider rejecting reserved/empty names.
- If you want to be more permissive, at least forbid `/`, `{`, `}`, and whitespace.
- (Optional) add a unit test for invalid names (e.g. `"contacts/admin"`, `"{id}"`, `""`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread tests/test_data_resource.py Outdated
Comment thread src/supervaizer/data_resource.py Outdated
Comment thread src/supervaizer/data_routes.py Outdated
Comment thread src/supervaizer/data_routes.py
Comment thread src/supervaizer/job.py
Comment thread src/supervaizer/data_routes.py
Comment on lines +162 to +166
def _make_import_handler(r: DataResource, prefix: str) -> Any:
async def _handler(records: list[dict[str, Any]] = Body(...)) -> dict[str, Any]:
log.info(f"📥 POST {prefix}/import/ [DataResource import: {r.name}]")
return r.on_import(records) # type: ignore[misc]
return _handler

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

7. Csv import not supported 🐞 Bug ≡ Correctness

The CHANGELOG claims the bulk import route accepts CSV or JSON, but the import handler only accepts
a JSON body (list[dict]); CSV uploads will fail validation.
Agent Prompt
### Issue description
The bulk import route is documented as accepting CSV or JSON, but the handler only accepts JSON (`records: list[dict[str, Any]] = Body(...)`). CSV content will not be accepted.

### Issue Context
This is an API contract issue. Either implement CSV parsing or update the documentation to remove CSV claims.

### Fix Focus Areas
- src/supervaizer/data_routes.py[97-105]
- src/supervaizer/data_routes.py[162-166]
- docs/CHANGELOG.md[38-39]

### Suggested fix
Option A (implement CSV):
- Accept `UploadFile` (multipart/form-data) or raw `text/csv` body.
- Parse CSV into `list[dict[str, Any]]` (header row -> keys), then call `on_import(records)`.
- Keep existing JSON support.

Option B (docs-only):
- Update CHANGELOG and any docs to state JSON-only import until CSV is implemented.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

…lity and consistency- long import lists and DataResourceField instantiations across multiple lines to make diffs clearer and improve maintainability.

- Break long Field(...) and model attribute declarations into multiple lines so descriptions and default factories are visually distinct.
- Add and blank in data_routes separate logical blocks.
- Annotate signature multiple lines for clarity.
- Adjust slicing and HTTPException formatting to conform to the project's spacing and line-length style.

These are purely stylist and do not alter behavior They make code easier to review and maintain, and keep lines within theproject's preferred width.
…Add a strict name pattern for DataResource (lowercase letters, digits,

and hyphens; must start with a letter or digit) and apply it to the DataResource.name field. This ensures path and OpenAPI operation_id fragments remain safe and predictable.
- Clarify and expand the DataResource.name Field description to document the allowed characters intent.
- Adjust on_update signature to allow returning None from update handlers (Callable[[str, dict[str, Any]], dict[str, Any] | None to support handlers that may not always return an updated object.
- Serialize event payloads in tests with SvBaseModel.serialize_value to match the application's payload serialization when sending events- Add a new test ensuring OpenAPI operation for data resource list endpoints unique per agent. This prevents collisions multiple agents expose resources with the same name.
- Update and test scaffolding to support the new test (add DataResource and Parameters imports RSA key generation in the test server setup).

These changes improve API, flexibility and test coveragefor OpenAPI uniqueness.
@alain-sv
alain-sv merged commit 0c10115 into develop Apr 14, 2026
6 checks passed
@alain-sv
alain-sv deleted the data_resources branch May 13, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant