WS-ART-001-03B4: continue verified guide sufficiency - #240
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds an artifact-owned material boundary, exact extraction validation, canonical 12 MiB prompt assembly, setup-generation fencing, provenance-backed report persistence, incident mapping, and verified guide-sufficiency execution. ChangesGuide sufficiency continuation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectSetupWorker
participant ProjectService
participant MaterialAdapter
participant AgentRuntime
participant ReportDatabase
ProjectSetupWorker->>ProjectService: Run verified sufficiency with setup generation
ProjectService->>MaterialAdapter: Load current verified material
MaterialAdapter-->>ProjectService: Return canonical material and provenance
ProjectService->>AgentRuntime: Submit bounded untrusted material
AgentRuntime-->>ProjectService: Return sufficiency result
ProjectService->>ReportDatabase: Persist report and source usages
ProjectService-->>ProjectSetupWorker: Return success or blocked outcome
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
70acd49 to
055e2db
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/tests/test_projects.py (1)
2388-2394: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix
enqueue_failureintest_create_source_snapshot_returns_created_when_post_commit_enqueue_fails
enqueue_pre_submit_setup_pipelinecalls this stub withsetup_generation, but the localenqueue_failureatbackend/tests/test_projects.py:2582-2588still declares only four keyword parameters and will raiseTypeError. Addsetup_generation: intto this stub signature, or use the updated local failure stubs from the other enqueue-failure tests.🤖 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_projects.py` around lines 2388 - 2394, Update the local enqueue_failure stub used by test_create_source_snapshot_returns_created_when_post_commit_enqueue_fails to accept the setup_generation: int keyword parameter passed by enqueue_pre_submit_setup_pipeline, matching the other enqueue-failure stubs while preserving its existing behavior.backend/app/modules/projects/setup_queue.py (1)
22-53: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHandle legacy setup queue messages with four positional arguments.
run_pre_submit_setup_pipelinenow requiressetup_generation, but the broker may still contain messages created by the previous task signature. Add a compatible default or run a queue purge/drain before rollout so workers fail messages with only four positional arguments and setup runs do not remain stuck.Also add
setup_generationto theenqueue_pre_submit_setup_pipelinedocstringArgsblock.🤖 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/projects/setup_queue.py` around lines 22 - 53, The enqueue path must remain compatible with legacy broker messages that contain four positional arguments: update the worker entry point run_pre_submit_setup_pipeline to accept a safe default for setup_generation (or otherwise drain the queue before deployment), ensuring old setup runs do not remain stuck. Also document setup_generation in the Args section of enqueue_pre_submit_setup_pipeline.
🧹 Nitpick comments (11)
backend/tests/test_guide_bindings.py (2)
393-406: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the setup-run output reference and the material byte count.
The locked design requires that the report, the provenance children, and the setup-run output reference commit together. This test verifies the report and the child usage row, but it does not verify that
ProjectSetupRun.output_sufficiency_report_idnow points at the new report. That field is the third member of the atomic write, andrun_verified_guide_sufficiency_agentsets it just before commit.Add that assertion, and assert
report.agent_material_byte_countequals the prompt length that the service hashed.💚 Proposed added assertions
assert usage.canonical_output_sha256 == output_digest + async with factory() as session: + persisted_run = await session.get(ProjectSetupRun, str(ids["run"])) + assert persisted_run is not None + assert persisted_run.output_sufficiency_report_id == report.id + assert report.agent_material_byte_count > 0🤖 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_guide_bindings.py` around lines 393 - 406, Extend the test assertions after report creation to verify report.agent_material_byte_count equals the prompt length used by the service, and load the relevant ProjectSetupRun to assert its output_sufficiency_report_id references report.id. Keep the existing provenance usage assertions unchanged and cover both atomic-write outputs.
196-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeed a second, obsolete extraction so the test proves its name.
The test is named
test_sufficiency_material_uses_only_exact_current_extraction, but it seeds exactly one extraction lineage. The assertionlen(result.source_items) == 1at Line 260 then holds because only one row exists, not because the adapter selected the policy-current row.The Python-side filter at Line 177 in
backend/app/modules/artifacts/guide_sufficiency_material.pyis the logic under test. Add a secondGuideSourceExtractedContentandGuideSourceExtractionUsagefor the same item with an obsoletepolicy_version, then assert thatresult.provenance[0].extraction_usage_idis the current usage id. That change exercises the filter and would fail if the filter were removed.🤖 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_guide_bindings.py` around lines 196 - 264, Extend test_sufficiency_material_uses_only_exact_current_extraction with a second obsolete GuideSourceExtractedContent and GuideSourceExtractionUsage for the same source item, using a non-current policy_version and distinct IDs while preserving the current lineage. Keep the existing single-result assertion and verify provenance selects the current usage_id, so the test exercises the policy-version filter rather than merely counting one row.backend/app/modules/artifacts/guide_sufficiency_material.py (1)
107-176: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider one set-based query instead of one locked query per item.
The loop issues a separate six-table locked
SELECT ... FOR UPDATEfor every snapshot item. The number of round trips and lock acquisitions grows linearly with the item count inside a single transaction. The lock set is identical to a single query that filters onGuideSourceArtifactBinding.source_item_id.in_(item_ids).If snapshot item counts stay small, the current form is acceptable and easier to read. If they can grow, replace the loop with one query and group the rows by
source_item_idin Python, then keep the existing per-item completeness and policy-currency checks.🤖 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/guide_sufficiency_material.py` around lines 107 - 176, Replace the per-item locked query in the loop with one set-based six-table SELECT using source_item_id.in_(item_ids), preserving all existing filters and with_for_update targets. Group the returned rows by source_item_id, then run the existing per-item completeness and policy-currency checks against each item’s grouped rows..agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md (1)
131-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude the new artifacts adapter in the focused coverage gate.
The acceptance criteria require at least 90% coverage for changed subsystems. The focused coverage gate that follows this command limits
--includetoapp/modules/projects/*and the setup worker. The new modulebackend/app/modules/artifacts/guide_sufficiency_material.pyis a changed subsystem, so its coverage is not measured by that gate.Add the new adapter path to the focused include list.
♻️ Suggested include-list change (line 133)
-(cd backend && .venv/bin/coverage report --include='app/modules/projects/*,app/*ers/project_setup.py' --precision=2 --fail-under=90) +(cd backend && .venv/bin/coverage report --include='app/modules/projects/*,app/modules/artifacts/guide_sufficiency_material.py,app/*ers/project_setup.py' --precision=2 --fail-under=90)As per coding guidelines: "New or materially changed backend subsystems must maintain at least 90% test coverage".
🤖 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/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md at line 131, Update the focused coverage gate following the test command to include app/modules/artifacts/guide_sufficiency_material.py alongside the existing app/modules/projects/* and setup worker paths. Preserve the current coverage threshold and all other command options.Source: Coding guidelines
backend/app/modules/projects/models.py (1)
687-692: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConstrain the new provenance columns at the schema level.
The four new columns accept any value.
setup_generationaccepts0and negative numbers.agent_material_sha256accepts a malformed digest.agent_material_byte_countaccepts a negative count. The columns are also all-or-nothing in practice: an agent report sets all four and a manual report sets none, but the schema allows any partial combination. A partially populated row would claim a setup run without a material digest.Sibling tables already enforce these invariants.
ProjectSetupRunusesck_project_setup_runs_generation_positive,GuideSourceExtractedContentusesSHA256_CHECK, and the newGuideSufficiencyReportSourceUsageusesck_sufficiency_report_usage_generation.Add matching constraints to
GuideSufficiencyReport, and add aSHA256_CHECKforGuideSufficiencyReportSourceUsage.canonical_output_sha256at Line 750. Add the same constraints tobackend/alembic/versions/0046_guide_sufficiency_provenance.py.♻️ Proposed constraints for `GuideSufficiencyReport.__table_args__`
UniqueConstraint( "source_snapshot_id", name="uq_guide_sufficiency_reports_source_snapshot", ), + CheckConstraint( + "setup_generation is null or setup_generation > 0", + name="ck_guide_sufficiency_reports_generation_positive", + ), + CheckConstraint( + "agent_material_byte_count is null or agent_material_byte_count >= 0", + name="ck_guide_sufficiency_reports_material_size", + ), + CheckConstraint( + "(project_setup_run_id is null and setup_generation is null " + "and agent_material_sha256 is null and agent_material_byte_count is null) or " + "(project_setup_run_id is not null and setup_generation is not null " + "and agent_material_sha256 is not null and agent_material_byte_count is not null)", + name="ck_guide_sufficiency_reports_material_provenance_shape", + ),🤖 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/projects/models.py` around lines 687 - 692, Add schema constraints to GuideSufficiencyReport.__table_args__ requiring positive setup_generation, a valid SHA-256 agent_material_sha256, nonnegative agent_material_byte_count, and all-or-nothing population of the four provenance columns. Add the corresponding constraints to revision 0046_guide_sufficiency_provenance.py, and add the existing SHA256_CHECK constraint pattern to GuideSufficiencyReportSourceUsage.canonical_output_sha256.backend/app/workers/project_setup.py (1)
50-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the new
setup_generationargument.The
Argsblock lists four arguments. The task now accepts five. Addsetup_generationto the docstring.🤖 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/workers/project_setup.py` around lines 50 - 74, Update the docstring for run_pre_submit_setup_pipeline to include setup_generation in the Args section, describing it as the setup generation associated with this pipeline run.backend/tests/test_projects.py (1)
2129-2138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWiden the captured-payload annotation.
enqueuedis annotatedlist[dict[str, str]], but the dictionary now stores anintforsetup_generation. Change the annotation tolist[dict[str, object]]so type checking stays accurate.🤖 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_projects.py` around lines 2129 - 2138, Update the enqueued annotation in the queue-capture helper to list[dict[str, object]], since its payload includes the integer setup_generation alongside string values.backend/app/modules/projects/guide_mutation_router.py (1)
104-117: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueCheck the generation invariant before the commit.
_finishcommits at Line 105 and raisesRuntimeErrorat Line 109. If the invariant ever breaks, the snapshot and setup run stay durable, the client receives a 500, and the setup run remainsqueuedwith no dispatch and no ledger error. Validate the outcome first so the failure cannot straddle the commit.♻️ Proposed reorder of the invariant check
async def _finish(session, outcome): + if outcome.setup_run_id and not outcome.replayed and outcome.setup_generation is None: + raise RuntimeError("committed project setup generation is unavailable") await (session.rollback() if outcome.replayed else session.commit()) if outcome.setup_run_id and not outcome.replayed: snapshot = outcome.response - if outcome.setup_generation is None: - raise RuntimeError("committed project setup generation is unavailable") await dispatch_pre_submit_setup_pipeline_after_commit(🤖 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/projects/guide_mutation_router.py` around lines 104 - 117, Update _finish to validate outcome.setup_generation before performing the commit or rollback, raising the existing RuntimeError when a non-replayed setup outcome lacks its generation. Preserve the current dispatch arguments and ensure valid outcomes retain the existing transaction and post-commit behavior.backend/tests/test_artifact_architecture.py (1)
206-213: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBroaden the boundary check to all artifact persistence imports.
The test matches only
ast.ImportFromwithnode.module == "app.modules.artifacts.models". Three bypasses remain:import app.modules.artifacts.models, a relative import inside the package, and any other artifact persistence module such as a repository. The enforcement then passes while the closed boundary is broken.♻️ Proposed broader enforcement
def test_project_domain_does_not_query_artifact_persistence_models() -> None: """Require project orchestration to consume the narrow material port.""" + forbidden_prefix = "app.modules.artifacts" violations: list[str] = [] for path in _python_files(APP_ROOT / "modules" / "projects"): for node in ast.walk(_tree(path)): - if isinstance(node, ast.ImportFrom) and node.module == "app.modules.artifacts.models": - violations.append(str(path.relative_to(BACKEND_ROOT))) + modules: list[str] = [] + if isinstance(node, ast.ImportFrom) and node.module is not None: + modules.append(node.module) + if isinstance(node, ast.Import): + modules.extend(alias.name for alias in node.names) + if any( + module == forbidden_prefix or module.startswith(f"{forbidden_prefix}.") + for module in modules + ): + violations.append(str(path.relative_to(BACKEND_ROOT))) assert violations == []🤖 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_architecture.py` around lines 206 - 213, Broaden test_project_domain_does_not_query_artifact_persistence_models to detect every artifact persistence import, including absolute imports, relative imports, and persistence submodules such as repositories. Inspect both Import and ImportFrom AST nodes, normalize relative module paths as needed, and continue asserting that no matching imports occur under the projects module.backend/app/modules/projects/service.py (1)
769-943: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSplit the verified continuation into named steps.
The method spans about 175 lines and mixes six concerns: request construction, material projection, canonical hashing, idempotent report lookup, agent invocation, and provenance persistence. The two idempotency blocks at Lines 841-852 and Lines 874-885 are identical apart from the digest variable. Extract a private helper for the material projection and a second helper for the "existing report matches this provenance" decision. The change is behavior-preserving and makes the two-pass verification easier to follow.
🤖 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/projects/service.py` around lines 769 - 943, Refactor run_verified_guide_sufficiency_agent into named steps by extracting the GuideSourceItemMaterial/GuideSourceMaterial construction into a private material-projection helper and the repeated existing-report provenance decision into a private lookup helper parameterized by the material digest. Preserve both-pass hashing, idempotent response behavior, rollback handling, and conflict errors exactly as-is.backend/app/adapters/project_agents/openai_agent_sdk.py (1)
233-243: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConvert serialization failures into
ProjectAgentRuntimeError.
json.dumps(..., allow_nan=False)raisesValueErrorfor non-finite floats. This call sits outside thetryblock, so aValueErrorescapes the adapter instead of the port-owned error type. Callers only handleProjectAgentRuntimeError.♻️ Proposed wrap of the serialization step
- prompt_bytes = ( - canonical_guide_source_material_bytes(material) - if isinstance(material, GuideSourceMaterial) - else json.dumps( - material, - sort_keys=True, - separators=(",", ":"), - ensure_ascii=False, - allow_nan=False, - ).encode("utf-8") - ) + try: + prompt_bytes = ( + canonical_guide_source_material_bytes(material) + if isinstance(material, GuideSourceMaterial) + else json.dumps( + material, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + allow_nan=False, + ).encode("utf-8") + ) + except ValueError: + raise ProjectAgentRuntimeError( + "OpenAI Agents SDK prompt is not canonically serializable" + ) from None🤖 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/adapters/project_agents/openai_agent_sdk.py` around lines 233 - 243, Move the non-GuideSourceMaterial serialization in the prompt construction around `prompt_bytes` into the existing `try` handling in the adapter, and translate `json.dumps` `ValueError` failures from `allow_nan=False` into `ProjectAgentRuntimeError`. Preserve the canonical guide serialization path and ensure callers never receive the raw serialization exception.
🤖 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/modules/artifacts/guide_sufficiency_material.py`:
- Around line 192-199: Update the IMAGE_FORMATS parsing branch in the guide
sufficiency material flow to validate that the result of json.loads assigned to
structural is a non-null dictionary/object. Raise
GuideSufficiencyMaterialUnavailable("guide_source_malformed") for arrays,
scalars, null, or invalid JSON, while preserving canonical clearing for valid
object metadata.
- Around line 62-92: The pre-lock latest_generation query can miss a generation
created before the locked transaction commits. In the header-loading flow around
lock_project_guide(), remove that standalone scalar read and derive the
generation from the locked ProjectSetupRun returned by the header query,
comparing its setup_generation with request.setup_generation while preserving
the existing stale-material exception.
In `@backend/app/workers/project_setup.py`:
- Around line 303-312: Update the ProjectServiceError handling around
run_verified_guide_sufficiency_agent to preserve distinct stable error codes:
map PolicySetupConflict to the guide-material-changed code and
PolicySetupBlocked to the verified-guide-unavailable/configuration code, while
retaining guide_source_stale only for the applicable stale-source case. Ensure
the persisted setup run status reflects the exact domain outcome.
- Around line 286-319: Add a final sanitized `except Exception` handler
alongside the existing `GuideSufficiencyMaterialUnavailable` and
`ProjectServiceError` branches in the setup pipeline. Roll back the session,
mark the setup run as terminal `setup_blocked` at the current guide-sufficiency
step with the established generic error code and summary used by
`_run_pre_submit_setup_pipeline`, and return the same sanitized response shape
without exposing the exception text.
In `@backend/tests/test_guide_bindings.py`:
- Around line 131-193: Ensure the downgraded database is restored to head in a
finally block around the assertions following command.downgrade in the migration
test. Keep engine disposal in the outer cleanup and make command.upgrade(config,
"head") execute even when schema or constraint assertions fail, while preserving
the existing assertions and cleanup behavior.
---
Outside diff comments:
In `@backend/app/modules/projects/setup_queue.py`:
- Around line 22-53: The enqueue path must remain compatible with legacy broker
messages that contain four positional arguments: update the worker entry point
run_pre_submit_setup_pipeline to accept a safe default for setup_generation (or
otherwise drain the queue before deployment), ensuring old setup runs do not
remain stuck. Also document setup_generation in the Args section of
enqueue_pre_submit_setup_pipeline.
In `@backend/tests/test_projects.py`:
- Around line 2388-2394: Update the local enqueue_failure stub used by
test_create_source_snapshot_returns_created_when_post_commit_enqueue_fails to
accept the setup_generation: int keyword parameter passed by
enqueue_pre_submit_setup_pipeline, matching the other enqueue-failure stubs
while preserving its existing behavior.
---
Nitpick comments:
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md:
- Line 131: Update the focused coverage gate following the test command to
include app/modules/artifacts/guide_sufficiency_material.py alongside the
existing app/modules/projects/* and setup worker paths. Preserve the current
coverage threshold and all other command options.
In `@backend/app/adapters/project_agents/openai_agent_sdk.py`:
- Around line 233-243: Move the non-GuideSourceMaterial serialization in the
prompt construction around `prompt_bytes` into the existing `try` handling in
the adapter, and translate `json.dumps` `ValueError` failures from
`allow_nan=False` into `ProjectAgentRuntimeError`. Preserve the canonical guide
serialization path and ensure callers never receive the raw serialization
exception.
In `@backend/app/modules/artifacts/guide_sufficiency_material.py`:
- Around line 107-176: Replace the per-item locked query in the loop with one
set-based six-table SELECT using source_item_id.in_(item_ids), preserving all
existing filters and with_for_update targets. Group the returned rows by
source_item_id, then run the existing per-item completeness and policy-currency
checks against each item’s grouped rows.
In `@backend/app/modules/projects/guide_mutation_router.py`:
- Around line 104-117: Update _finish to validate outcome.setup_generation
before performing the commit or rollback, raising the existing RuntimeError when
a non-replayed setup outcome lacks its generation. Preserve the current dispatch
arguments and ensure valid outcomes retain the existing transaction and
post-commit behavior.
In `@backend/app/modules/projects/models.py`:
- Around line 687-692: Add schema constraints to
GuideSufficiencyReport.__table_args__ requiring positive setup_generation, a
valid SHA-256 agent_material_sha256, nonnegative agent_material_byte_count, and
all-or-nothing population of the four provenance columns. Add the corresponding
constraints to revision 0046_guide_sufficiency_provenance.py, and add the
existing SHA256_CHECK constraint pattern to
GuideSufficiencyReportSourceUsage.canonical_output_sha256.
In `@backend/app/modules/projects/service.py`:
- Around line 769-943: Refactor run_verified_guide_sufficiency_agent into named
steps by extracting the GuideSourceItemMaterial/GuideSourceMaterial construction
into a private material-projection helper and the repeated existing-report
provenance decision into a private lookup helper parameterized by the material
digest. Preserve both-pass hashing, idempotent response behavior, rollback
handling, and conflict errors exactly as-is.
In `@backend/app/workers/project_setup.py`:
- Around line 50-74: Update the docstring for run_pre_submit_setup_pipeline to
include setup_generation in the Args section, describing it as the setup
generation associated with this pipeline run.
In `@backend/tests/test_artifact_architecture.py`:
- Around line 206-213: Broaden
test_project_domain_does_not_query_artifact_persistence_models to detect every
artifact persistence import, including absolute imports, relative imports, and
persistence submodules such as repositories. Inspect both Import and ImportFrom
AST nodes, normalize relative module paths as needed, and continue asserting
that no matching imports occur under the projects module.
In `@backend/tests/test_guide_bindings.py`:
- Around line 393-406: Extend the test assertions after report creation to
verify report.agent_material_byte_count equals the prompt length used by the
service, and load the relevant ProjectSetupRun to assert its
output_sufficiency_report_id references report.id. Keep the existing provenance
usage assertions unchanged and cover both atomic-write outputs.
- Around line 196-264: Extend
test_sufficiency_material_uses_only_exact_current_extraction with a second
obsolete GuideSourceExtractedContent and GuideSourceExtractionUsage for the same
source item, using a non-current policy_version and distinct IDs while
preserving the current lineage. Keep the existing single-result assertion and
verify provenance selects the current usage_id, so the test exercises the
policy-version filter rather than merely counting one row.
In `@backend/tests/test_projects.py`:
- Around line 2129-2138: Update the enqueued annotation in the queue-capture
helper to list[dict[str, object]], since its payload includes the integer
setup_generation alongside string values.
🪄 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: aba8f05a-280b-4b5d-ae8d-a6304f6bed32
📒 Files selected for processing (24)
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B4-pr-trust-bundle.mdbackend/alembic/versions/0046_guide_sufficiency_provenance.pybackend/app/adapters/project_agents/openai_agent_sdk.pybackend/app/interfaces/artifact_operations.pybackend/app/interfaces/project_agents.pybackend/app/modules/artifacts/guide_sufficiency_material.pybackend/app/modules/artifacts/models.pybackend/app/modules/projects/guide_mutation_router.pybackend/app/modules/projects/guide_mutation_service.pybackend/app/modules/projects/models.pybackend/app/modules/projects/schemas.pybackend/app/modules/projects/service.pybackend/app/modules/projects/setup_queue.pybackend/app/workers/project_setup.pybackend/tests/conftest.pybackend/tests/test_artifact_architecture.pybackend/tests/test_guide_bindings.pybackend/tests/test_projects.pydocs/architecture_data_model.mddocs/spec_artifact_storage_service.md
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 `@backend/app/interfaces/artifact_operations.py`:
- Around line 34-41: Update the `__all__` declaration in
`artifact_operations.py` to include `GuideSufficiencySourceItem`,
`GuideSufficiencyExtractionProvenance`, and
`GuideSufficiencyMaterialUnavailable` alongside the existing guide sufficiency
symbols, preserving the public export list’s organization.
🪄 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: abf9b3d5-214b-44cf-bec8-94b2af36dc51
📒 Files selected for processing (24)
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B4-pr-trust-bundle.mdbackend/alembic/versions/0046_guide_sufficiency_provenance.pybackend/app/adapters/project_agents/openai_agent_sdk.pybackend/app/interfaces/artifact_operations.pybackend/app/interfaces/project_agents.pybackend/app/modules/artifacts/guide_sufficiency_material.pybackend/app/modules/artifacts/models.pybackend/app/modules/projects/guide_mutation_router.pybackend/app/modules/projects/guide_mutation_service.pybackend/app/modules/projects/models.pybackend/app/modules/projects/schemas.pybackend/app/modules/projects/service.pybackend/app/modules/projects/setup_queue.pybackend/app/workers/project_setup.pybackend/tests/conftest.pybackend/tests/test_artifact_architecture.pybackend/tests/test_guide_bindings.pybackend/tests/test_projects.pydocs/architecture_data_model.mddocs/spec_artifact_storage_service.md
🚧 Files skipped from review as they are similar to previous changes (20)
- docs/architecture_data_model.md
- .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md
- backend/app/modules/projects/schemas.py
- .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md
- backend/app/modules/projects/guide_mutation_service.py
- backend/tests/test_artifact_architecture.py
- .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B4-pr-trust-bundle.md
- docs/spec_artifact_storage_service.md
- backend/app/modules/projects/models.py
- .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md
- backend/app/modules/artifacts/guide_sufficiency_material.py
- backend/app/modules/projects/setup_queue.py
- .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md
- backend/tests/conftest.py
- backend/app/modules/artifacts/models.py
- backend/app/modules/projects/service.py
- backend/tests/test_guide_bindings.py
- backend/app/workers/project_setup.py
- backend/tests/test_projects.py
- backend/app/modules/projects/guide_mutation_router.py
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 `@backend/app/adapters/project_agents/openai_agent_sdk.py`:
- Around line 233-248: Update the serialization error handler around
prompt_bytes construction to catch both TypeError and ValueError from
json.dumps, while preserving the existing ProjectAgentRuntimeError and message.
Add a regression test covering an unsupported value or incompatible sorted keys
and verify it raises ProjectAgentRuntimeError.
🪄 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: af2c277e-f6e1-425e-a1df-2ecb76c65b11
📒 Files selected for processing (15)
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B4-external-review-response.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B4-pr-trust-bundle.mdbackend/alembic/versions/0046_guide_sufficiency_provenance.pybackend/app/adapters/project_agents/openai_agent_sdk.pybackend/app/interfaces/artifact_operations.pybackend/app/modules/artifacts/guide_sufficiency_material.pybackend/app/modules/projects/guide_mutation_router.pybackend/app/modules/projects/models.pybackend/app/modules/projects/setup_queue.pybackend/app/workers/project_setup.pybackend/tests/conftest.pybackend/tests/test_artifact_architecture.pybackend/tests/test_guide_bindings.pybackend/tests/test_projects.py
🚧 Files skipped from review as they are similar to previous changes (10)
- backend/app/modules/projects/guide_mutation_router.py
- backend/tests/test_artifact_architecture.py
- backend/app/modules/projects/models.py
- .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md
- backend/tests/conftest.py
- backend/app/modules/projects/setup_queue.py
- backend/tests/test_guide_bindings.py
- backend/app/workers/project_setup.py
- backend/alembic/versions/0046_guide_sufficiency_provenance.py
- backend/tests/test_projects.py
2 participants
WS-ART-001-03B4 PR Trust Bundle
Chunk
WS-ART-001-03B4— Guide Sufficiency Continuation (L1).Goal and human-approved intent
Feed only complete, verified, same-generation canonical guide material into
the existing asynchronous sufficiency workflow and persist the exact ART
lineage consumed. Keep the behavior hidden and binding/read AUTH actions
planned and unavailable until AUTH-04B.
What changed and why
content, classification, extraction, setup-run, and generation lineage.
generation payload.
every source item as untrusted, and sends the exact hashed bytes.
per consumed extraction usage.
without misclassifying them as guide insufficiency.
Design chosen and alternatives rejected
ART owns persistence joins behind
GuideSufficiencyMaterialPort; project codeconsumes typed immutable DTOs. The worker reloads durable identifiers and exact
generation facts. Rejected alternatives include ART model imports in project
services, raw binaries or caller excerpts in prompts, prepared AUTH handles in
Celery, generic download authority, incomplete source sets, and live legacy
cutover before AUTH-04B.
Scope control and product behavior
This is hidden guide setup behavior only. It does not activate AUTH actions,
replace the live legacy path, remove legacy identity, parse provider objects,
or change submission, review, contribution, compensation, or reputation flows.
ART-03C owns the later clean cut.
Acceptance criteria proof
policy-current extraction usage.
before invocation and again before atomic report commit.
12 MiB passes and one byte over fails before invocation.
provenance row; stale and crossed lineage fails closed.
Tests and checks run
canonical prompt, and queue/router tests passed in focused runs.
git diff --check— pass.to avoid the user's slow local machine.
Test delta and CI integrity
Tests add exact provenance, replay, generation, prompt-boundary, incident, and
migration proofs. No tests, assertions, workflows, lanes, or coverage floors
were removed or weakened. Repository 78 percent and changed-subsystem 90
percent requirements remain intact.
Reviewer results
Architecture, security, product/ops, QA, senior engineering, CI integrity, and
test-delta reviews pass with no blockers. Docs and reuse findings were repaired:
the data model/chunk map now match the schema, and canonical JSON rejects
non-finite values. Final rereviews are recorded before merge readiness.
External review
Hosted Backend/Agent Gates and CodeRabbit have not yet run on the final PR head.
Their valid findings will be addressed before merge readiness.
Remaining risks and follow-up work
The hidden continuation intentionally coexists with the legacy live path.
AUTH-04B must activate only fixed-service binding/read authority after this
chunk merges; ART-03C then performs the separate legacy cutover.
Human review focus and merge ownership
Review the all-items-required query, transaction/generation fences, exact
prompt identity and ceiling, normalized provenance constraints, replay behavior,
and absence of AUTH activation or legacy cutover. A human owns merge approval;
the agent will not merge this PR.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation