Source and presenter-profile intake (4.3.1) - #131
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (58)
👮 Files not reviewed due to content moderation or server errors (58)
Source and Presenter-Profile Intake: REST API ImplementationOverviewThis PR implements the initial REST surface for roadmap item 4.3.1 (source-and-presenter-profile intake for script generation), providing the domain models, application services, and HTTP endpoints for a source-intake vertical slice as specified in ADR 009. The work includes execution planning, architectural decision recording, comprehensive domain modelling, SQLAlchemy persistence layers, and end-to-end integration tests. Key Implementation ComponentsArchitectural Decisions:
Domain Models:
Application Services:
HTTP API Surface:
Persistence:
Documentation and Design Links:
Testing Strategy
Out-of-Scope DeferralsPer ADR 015 and the execution plan:
Configuration Changes
Related Documentation
WalkthroughAdd the intake ADR and execution plan, define intake and idempotency domain/storage contracts, wire the new API routes and handlers, update runtime fixtures, and add end-to-end validation for uploads, ingestion jobs, and replay/conflict handling. ChangesSource-intake architecture and implementation slice
Sequence Diagram(s)sequenceDiagram
participant Client
participant UploadsResource
participant SourceIntakeService
participant SqlAlchemyUnitOfWork
participant FilesystemObjectStore
participant SqlAlchemyIdempotencyStore
Client->>UploadsResource: POST /v1/uploads
UploadsResource->>SourceIntakeService: register_upload(...)
SourceIntakeService->>FilesystemObjectStore: put(...)
SourceIntakeService->>SqlAlchemyUnitOfWork: add Upload and commit
UploadsResource->>SqlAlchemyIdempotencyStore: acquire/complete replay state
UploadsResource->>Client: 201 upload response or replay/conflict
Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
e6bbbff to
c9dc016
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai resume |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== test_reference_document_bindings_facade.py ==="
cat tests/test_reference_document_bindings_facade.py
echo ""
echo "=== test_brief_loaders.py (first 30 lines) ==="
head -30 tests/test_brief_loaders.py
echo ""
echo "=== database.py - lock check ==="
grep -n "Lock\|_schema_reset_lock\|_reset_public_schema" tests/fixtures/database.py
echo ""
echo "=== ADR-015 - HTTP concepts check ==="
grep -n "route\|response_status\|response_body\|response_headers\|IdempotencyOutcome\|Acquired\|Replay\|Conflict\|InFlight\|serialised_outcome\|operation" docs/adr/adr-015-upload-and-idempotency-ports.md | head -60
echo ""
echo "=== Open issues ==="
gh issue list --repo leynos/episodic --state open --limit 50 --json number,title | jq '.[] | {number, title}'Length of output: 12245
Status of failed checks
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add a shared async reset lock around the py-pglite public schema reset fixture so concurrent callers cannot drop `public` during another reset. Cover the path with a fixture test that verifies both lock use and the post-reset schema state. Document the xdist serialisation rule, and make the runtime wiring readiness test run lifespan shutdown so the database engine is disposed before the next py-pglite fixture starts.
Add upload, source attachment, idempotency, and object-store domain contracts for the source-intake workflow. Include a filesystem-backed object-store adapter so local intake paths can persist and stream opaque upload payloads behind the new port. Cover the ADR 015 multipart fingerprint, object-store path and size guards, and idempotency replay/conflict invariants with focused tests. Record the published ADR vector discrepancy in the execplan so the compatibility value remains explicit.
Apply formatter-driven line wrapping to existing documentation touched by the validation workflow. Keep the changes textual only so Markdown lint and diagram validation remain clean.
Add Alembic-backed upload, source-attachment, and idempotency storage plus source-intake application services and /v1 Falcon resources. Cover SQLAlchemy repository round trips, domain idempotency outcomes, the ADR 015 multipart worked vector, a BDD source-intake scenario, a response-envelope snapshot, and the end-to-end upload/job/source/poll workflow.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Complex Methodepisodic/canonical/idempotency.py: post_init episodic/canonical/ingestion_sources.py: post_init episodic/canonical/uploads.py: post_init Why does this problem occur?A Complex Method has a high cyclomatic complexity. The recommended threshold for the Python language is a cyclomatic complexity lower than 9. How to fix it?There are many reasons for Complex Method. Sometimes, another design approach is beneficial such as a) modeling state using an explicit state machine rather than conditionals, or b) using table lookup rather than long chains of logic. In other scenarios, the function can be split using EXTRACT FUNCTION. Just make sure you extract natural and cohesive functions. Complex Methods can also be addressed by identifying complex conditional expressions and then using the DECOMPOSE CONDITIONAL refactoring. Helpful refactoring examplesTo get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes. SAMPLE# complex_method.js
function postItem(item) {
if (!item.id) {
- if (item.x != null && item.y != null) {
- post(item);
- } else {
- throw Error("Item must have x and y");
- }
+ // extract a separate function for creating new item
+ postNew(item);
} else {
- if (item.x < 10 && item.y > 25) {
- put(item);
- } else {
- throw Error("Item must have an x and y value between 10 and 25");
- }
+ // and one for updating existing items
+ updateItem(item);
}
}
+
+function postNew(item) {
+ validateNew(item);
+ post(item);
+}
+
+function updateItem(item) {
+ validateUpdate(item);
+ put(item);
+}
+ |
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Large Methodepisodic/api/app.py: create_app has 82 lines, threshold = 70 Why does this problem occur?Overly long functions make the code harder to read. The recommended maximum function length for the Python language is 70 lines of code. Severity: Brain Method - Complex Method - Long Method. How to fix it?We recommend to be careful here -- just splitting long functions don't necessarily make the code easier to read. Instead, look for natural chunks inside the functions that expresses a specific task or concern. Often, such concerns are indicated by a Code Comment followed by an if-statement. Use the EXTRACT FUNCTION refactoring to encapsulate that concern. |
Document the origin/main rebase conflict resolution in the source-intake execplan and remove a Markdown blank-line lint issue exposed after the rebase.
38a45c4 to
c5d59f7
Compare
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Code Duplicationepisodic/api/source_idempotency.py: The module contains 2 functions with similar structure: _idempotency_conflict,_idempotency_in_flight episodic/canonical/storage/source_intake_mappers.py: The module contains 4 functions with similar structure: _ingestion_job_source_from_record,_ingestion_job_source_to_record,_upload_from_record,_upload_to_record Why does this problem occur?Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health. How to fix it?A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Move idempotency, ingestion-source, and upload dataclass validation branches into private module-level helpers so the __post_init__ methods stay as flat delegation sequences without changing validation semantics.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Share idempotency conflict construction and metadata deep-copy mapping through private helpers. Preserve the existing HTTP error payloads and mapper copy semantics while keeping the mapper helpers type-preserving.
|
@coderabbitai resume |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Move Falcon route registration groups and the source-intake BDD API call sequence into private helpers. Keep route strings, resource construction, and test assertions unchanged while reducing large-method complexity.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Code Duplicationepisodic/canonical/storage/source_intake_mappers.py: What lead to degradation?The module contains 4 functions with similar structure: _ingestion_job_source_from_record,_ingestion_job_source_to_record,_upload_from_record,_upload_to_record Why does this problem occur?Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health. How to fix it?A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More |
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as resolved.
This comment was marked as resolved.
1 participant
Summary
docs/execplans/4-3-1-source-and-presenter-profile-intake-script-generation.md, the execution plan for roadmap item 4.3.1, the first half of the source-to-script vertical slice from ADR 009.POST /v1/uploads,POST /v1/ingestion-jobs,POST /v1/ingestion-jobs/{job_id}/sourcesand theirGETpairs), anObjectStorePortwith a local-filesystem adapter, anIdempotencyStorePortbacked by a SQL unique constraint, and a newIngestionJob.intake_statecolumn kept orthogonal to the existing multi-source mergestatus.awaiting_sources → ready_for_generationtransition, and a two-phase blob-write pattern that keeps the multipart path safe under rollback.Notes for reviewers
DRAFTExecPlan that follows the conventions in theexecplansskill.POST /v1/uploads/init+PUT bytes), magic-byte sniffing, S3-compatible adapters, and Vidai Mock fixtures — each with an explicit deferral target. See theDecision logandRiskssections.Revision historysection at the bottom.hexagonal-architecture,python-routerand its follow-ons,python-testing,python-verification,hypothesis,execplans,leta,commit-message,en-gb-oxendict) and the relevant documentation (ADR 009, ADR 014,docs/episodic-tui-api-design.md,docs/async-sqlalchemy-with-pg-and-falcon.md,docs/testing-async-falcon-endpoints.md,docs/testing-sqlalchemy-with-pytest-and-py-pglite.md).Test plan
domain_ports,application, andoutbound_adapter).npx markdownlint-cli2 docs/execplans/4-3-1-source-and-presenter-profile-intake-script-generation.md— passes locally.References
docs/execplans/4-3-1-source-and-presenter-profile-intake-script-generation.mddocs/roadmap.md§4.3.1docs/adr/adr-009-source-to-script-rest-vertical-slice.mddocs/adr/adr-014-hexagonal-architecture-enforcement.md