WS-ART-001-04A1: remove legacy contributor intake - #264
Merged
Conversation
📝 WalkthroughWalkthroughThis PR removes legacy contributor upload intake from artifact models, admission, repository, service, and routes. It adds migration ChangesArtifact intake removal
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant Migration0052
participant PostgreSQL
Operator->>Migration0052: Run upgrade
Migration0052->>PostgreSQL: Lock legacy intake tables
PostgreSQL-->>Migration0052: Check legacy rows and references
Migration0052->>PostgreSQL: Remove legacy schema if checks are empty
PostgreSQL-->>Operator: Commit revision 0052
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/tests/test_artifact_recovery.py (1)
63-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the shared checker-output helper into a helper module.
Line 65 imports the module-private
_admit_checker_outputfrom another test module. The repository already has a shared helper module,tests/artifact_store_helpers.py, used on line 64. A test-to-test import couples collection order betweentest_artifact_recovery.pyandtest_artifact_admission.py, and it re-executes the other module at import time.Move
_admit_checker_outputand its dependency_seed_checker_output_relationshipsinto a shared helper module, then import both test modules from there.♻️ Proposed import change after moving the helpers
from tests.artifact_store_helpers import artifact_admission_limit_settings, minted_source -from tests.test_artifact_admission import _admit_checker_output +from tests.artifact_checker_helpers import admit_checker_output🤖 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/tests/test_artifact_recovery.py` around lines 63 - 65, Move the shared helpers _admit_checker_output and _seed_checker_output_relationships from tests/test_artifact_admission.py into tests/artifact_store_helpers.py, preserving their behavior and dependencies. Update both test_artifact_admission.py and test_artifact_recovery.py to import these helpers from artifact_store_helpers, and remove the test-to-test import and duplicate definitions.backend/app/modules/artifacts/models.py (1)
844-856: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign
put_attempt_idnullability with the CHECK.
contract_producer_referencerequiresput_attempt_id is not null, butArtifactOperationReceipt.put_attempt_idis still declared asMapped[str | None]withoutnullable=False. Addnullable=Falseand change the type toMapped[str]with a matching Alembic alteration once a live not-null transition 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/modules/artifacts/models.py` around lines 844 - 856, Update ArtifactOperationReceipt.put_attempt_id to Mapped[str] with nullable=False so its SQLAlchemy declaration matches the contract_producer_reference CHECK constraint, and add the corresponding Alembic schema alteration for the database column.
🤖 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/tests/test_artifact_recovery.py`:
- Around line 341-345: Remove all test assignments and reads of the unmapped
ArtifactVerificationJob.recovery_submission_id in
backend/tests/test_artifact_recovery.py at lines 341-345, 362-366, 501, and 749;
use the corresponding recovery attempt’s submission_id when constructing or
validating recovery requests, preserving the existing recovery flow.
---
Nitpick comments:
In `@backend/app/modules/artifacts/models.py`:
- Around line 844-856: Update ArtifactOperationReceipt.put_attempt_id to
Mapped[str] with nullable=False so its SQLAlchemy declaration matches the
contract_producer_reference CHECK constraint, and add the corresponding Alembic
schema alteration for the database column.
In `@backend/tests/test_artifact_recovery.py`:
- Around line 63-65: Move the shared helpers _admit_checker_output and
_seed_checker_output_relationships from tests/test_artifact_admission.py into
tests/artifact_store_helpers.py, preserving their behavior and dependencies.
Update both test_artifact_admission.py and test_artifact_recovery.py to import
these helpers from artifact_store_helpers, and remove the test-to-test import
and duplicate definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2329652d-a0cc-417d-9d99-f05fc6ab0599
📒 Files selected for processing (23)
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/DECISIONS.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/DISCOVERY.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04A1-legacy-contributor-intake-removal.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A1-internal-review-evidence.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A1-pr-trust-bundle.mdbackend/alembic/versions/0051_legacy_intake_removal.pybackend/app/db/models.pybackend/app/modules/artifacts/models.pybackend/app/modules/artifacts/repository.pybackend/app/modules/artifacts/schemas.pybackend/app/modules/artifacts/service.pybackend/tests/conftest.pybackend/tests/test_alembic.pybackend/tests/test_artifact_admission.pybackend/tests/test_artifact_architecture.pybackend/tests/test_artifact_authorization.pybackend/tests/test_artifact_recovery.pydocs/decision_0013_immutable_artifact_storage_boundary.mddocs/glossary.mddocs/operations_artifact_storage.mddocs/spec_artifact_storage_service.md
💤 Files with no reviewable changes (5)
- docs/glossary.md
- backend/tests/test_artifact_authorization.py
- backend/app/db/models.py
- backend/app/modules/artifacts/service.py
- backend/app/modules/artifacts/repository.py
…-legacy-intake-removal # Conflicts: # backend/tests/conftest.py # backend/tests/test_alembic.py
abiorh-claw
self-requested a review
August 4, 2026 07:55
abiorh-claw
approved these changes
Aug 4, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A1-pr-trust-bundle.md:
- Around line 20-21: Synchronize the acceptance proof’s migration revision with
the reconciled migration identified in the external review response: replace the
stale 0050 → 0051 → 0050 → 0051 sequence with 0052_legacy_intake_removal or the
established HEAD_REVISION symbol, while preserving the surrounding proof.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a9401505-bdc2-4a63-aba1-d79542a95e42
📒 Files selected for processing (11)
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A1-external-review-response.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A1-pr-trust-bundle.mdbackend/alembic/versions/0052_legacy_intake_removal.pybackend/app/db/models.pybackend/app/modules/artifacts/models.pybackend/tests/conftest.pybackend/tests/test_alembic.pybackend/tests/test_artifact_operator_api.pybackend/tests/test_artifact_recovery.pybackend/tests/test_artifacts.pybackend/tests/test_review_queue_persistence.py
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/app/db/models.py
- backend/tests/test_artifact_operator_api.py
- backend/app/modules/artifacts/models.py
Comment on lines
+20
to
+21
| - Added migration `0052_legacy_intake_removal`: exclusive-lock preflight, | ||
| atomic populated refusal, safe-empty removal, and exact empty downgrade. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Synchronize the migration revision in the acceptance proof.
This section names 0052_legacy_intake_removal. Line 42 still records 0050 -> 0051 -> 0050 -> 0051, while .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A1-external-review-response.md Lines 45-56 identifies 0052_legacy_intake_removal as the reconciled migration. Update the proof to use 0052_legacy_intake_removal or HEAD_REVISION. Otherwise, reviewers can validate the wrong migration path.
Proposed documentation fix
-- Empty `0050 -> 0051 -> 0050 -> 0051` round trip passed with exact physical
+- Empty `0050_guide_source_v2 -> HEAD_REVISION (0052_legacy_intake_removal) ->
+ 0050_guide_source_v2 -> HEAD_REVISION` round trip passed with exact physical🤖 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
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A1-pr-trust-bundle.md
around lines 20 - 21, Synchronize the acceptance proof’s migration revision with
the reconciled migration identified in the external review response: replace the
stale 0050 → 0051 → 0050 → 0051 sequence with 0052_legacy_intake_removal or the
established HEAD_REVISION symbol, while preserving the surrounding proof.
This was referenced Aug 4, 2026
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.
WS-ART-001-04A1 PR Trust Bundle
Chunk
WS-ART-001-04A1 — Legacy Contributor Intake Removal(L1)Goal and approved intent
Remove the inactive multi-step contributor upload-session/item path before the
one-outer-ZIP submission pipeline is built. Preserve historical evidence by
refusing populated deployments; do not translate, detach, delete, or fabricate
legacy lineage.
What changed and why
and dispatch, repository lookups, and shared mutation branches.
producers only.
0051_legacy_intake_removal: exclusive-lock preflight,atomic populated refusal, safe-empty removal, and exact empty downgrade.
task-scoped checker-output proofs.
guidance.
Design and rejected alternatives
Chosen: a complete safe-empty clean cut. Rejected: detached compatibility
columns, inferred backfill, automatic deletion, dual runtime paths, and adding
the replacement submission route in this chunk.
Scope and product behavior
No public contributor intake is added. Existing guide and checker storage,
verification, and recovery behavior remains available. A deployment with any
legacy intake evidence remains on
0050_guide_source_v2and requires separatelyapproved maintenance work.
Acceptance proof
0050 -> 0051 -> 0050 -> 0051round trip passed with exact physicalschema comparison.
surface.
concurrency, and recovery lineage tests passed.
CI and test integrity
No CI, threshold, dependency, or package-script changes. Ruff, stale scans,
Markdown links, and diff checks pass. Contributor-specific tests were removed;
their surviving generic guarantees were re-established on current producers.
Reviewer results and external review
Architecture, security, QA, product/operations, senior engineering,
documentation, reuse, and test-delta reviews pass. Hosted GitHub Backend/Agent
Gates and CodeRabbit remain pending when this bundle is written.
Remaining risks and follow-up
approved maintenance/audit work.
chunk; 04A1 creates no submission behavior.
Human review focus and merge ownership
Confirm the preflight predicates, exact downgrade shape, absence of a
replacement route, and the replacement checker/guide coverage. Human approval
is required for merge; the agent will not merge this PR.
Summary by CodeRabbit
Changes
Documentation