docs(arch): split submission capability repair - #313
Conversation
|
Warning Review limit reached
Next review available in: 1 minute You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (18)
📝 WalkthroughWalkthroughThis PR reorganizes submission-capability planning into WS-ARCH-001-02A–02I. It defines ownership and staged activation contracts, supersedes older ART/AUTH/XINT proposals, and updates architecture documentation for the deferred admission-only cutover. ChangesSubmission capability architecture
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 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-ARCH-001-modular-monolith-boundaries/CHUNK_MAP.md:
- Around line 16-20: Update the descriptions for WS-ARCH-001-03 through
WS-ARCH-001-07 in the chunk map so “requires split contract” becomes
grammatically complete, using “requires a split contract” or equivalent wording
while preserving the existing scope and meaning.
In
@.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02C-checker-pre-submit-api.md:
- Line 52: Replace the initiative-wide allowlist pattern with explicit paths for
this chunk’s contract and evidence files only, so WS-ARCH-001-02C cannot modify
sibling contracts, parent decisions, status files, or unrelated evidence. Add
any required coordination files as individually named entries, following the
PR-sized contract scope for chunks 02A–02I.
- Around line 64-70: Update the 02C CHECKER API contract so its immutable public
execution result excludes PreSubmissionExecutionResult.custody and all
PreSubmissionExecutionCustody fields, including storage_scheme. Keep custody
data in the ART-owned adapter, expose only bounded checker facts publicly, and
add a contract test that rejects custody fields from the public CHECKER result.
In
@.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02D-art-preparation-public-api.md:
- Line 53: Replace the initiative-wide glob allowlist with explicit paths for
the WS-ARCH-001-02D contract and its evidence artifacts, while separately naming
any required coordination or activation files. Update the allowlist entry in the
02D preparation document so sibling chunk contracts and unrelated shared records
remain excluded.
- Line 86: Replace the shell-unsafe <test-db> placeholder with an exported
WORKSTREAM_TEST_DATABASE_URL in every verification command:
.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02D-art-preparation-public-api.md:86-86,
.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02E-art-admission-binding-api.md:88-88,
and
.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02F-task-submission-composition.md:87-87.
Ensure each command exports the variable before invoking pytest and uses a
shell-safe database value.
In
@.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02E-art-admission-binding-api.md:
- Line 54: Update the initiative-wide allowlist entry for WS-ARCH-001 so it no
longer permits all files under the initiative. Restrict it to the 02E contract
and evidence paths, and explicitly list any required coordination files; do not
include sibling admission, activation, or cutover contracts.
- Around line 15-16: Define a typed TASK-owned lineage capability that returns
immutable task, assignment, predecessor-version, project, guide, snapshot, and
policy facts. Reference this port in the entry gate, acceptance criteria, and
resource manifest, and retain ART’s locked recheck until all TASK persistence
queries are replaced by the capability.
In
@.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02F-task-submission-composition.md:
- Line 54: Restrict the initiative-wide allowlist entry to only the
WS-ARCH-001-02F contract and evidence paths, removing the recursive permission
for every file under the broader WS-ARCH-001 initiative. Explicitly add any
required coordination files so sibling transaction, activation, and cutover
contracts remain outside the 02F scope.
- Around line 15-16: Update PreparedBundlePreSubmitEvidenceService.persist(...)
and its port/composition so evidence writes participate in the same root
transaction created by self._authority.transaction(), rather than opening and
committing independently. Ensure the Submission, binding, admission, and
evidence effects commit atomically while lifecycle ownership remains in the
existing domain boundary, and add an integration test proving a final-stage
failure rolls back all related rows.
In
@.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/reviews/WS-ARCH-001-02-plan-review.md:
- Around line 11-20: Normalize the Final result values in the review evidence
table to lowercase pass or fail, replacing PASS WITH LOW RISKS with pass and
moving its qualifier into Material correction or a separate risk field. Apply
this convention to internal engineering review evidence under
.agent-loop/initiatives/**/reviews/*.md, while preserving accept,
needs_revision, and reject for persisted product review decisions.
- Line 21: Update the “Test delta” table entry to replace “additive focused
regression and coverage proof” with a clear noun phrase describing the planned
regression tests and coverage evidence, without changing the surrounding status
or meaning.
In
@.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md:
- Line 98: Update the WS-XINT-002-05B entry in CHUNK_MAP.md to change “requires
hidden 02F transaction” to “requires the hidden 02F transaction,” leaving the
rest of the description unchanged.
In `@docs/architecture_checker_framework.md`:
- Around line 273-276: The legacy Submission-field cleanup ownership is
inconsistent across the two documents. In docs/architecture_checker_framework.md
lines 273-276, reconcile WS-ARCH-001-02I route/path removal with
WS-ART-001-05B’s existing field-removal responsibility and explicitly state that
artifact_hash is handled separately; in docs/architecture_data_model.md lines
1339-1342, align the legacy field list and artifact_hash transitional status
with that same ownership and cutover contract.
In `@docs/spec_authorization_service.md`:
- Around line 430-434: Update the activation chain around XINT-002-06A to
include ART-04B1, ART-04B2, and ART-04B3, or explicitly state their transitive
dependency before ART-04C1/04C2. Ensure the chain demonstrates that the
catalogue, materialization, and evidence path is ready before WS-ARCH-001-02G.
- Around line 478-480: Align the ownership mapping for
artifact.submission.binding.create across the authorization specification table
and the executable action registry. Update either the WS-ARCH-001-02H table
entry or the ActionId.ARTIFACT_SUBMISSION_BINDING_CREATE assignment in
catalogue.py so both identify the same AUTH activation custodian, preserving the
dedicated custodian’s authority to promote the action.
🪄 Autofix
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: ed4bcca6-1d72-4e5f-8183-95dbeb037949
📒 Files selected for processing (44)
.agent-loop/CURRENT_STATE.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/CHUNK_MAP.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/DECISIONS.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/DISCOVERY.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/PLAN.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/RISKS.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/STATUS.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02-submission-capability-repair.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02A-task-submission-context-api.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02B-project-locked-policy-api.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02C-checker-pre-submit-api.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02D-art-preparation-public-api.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02E-art-admission-binding-api.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02F-task-submission-composition.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02G-auth-preparation-activation.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02H-auth-consumption-activation.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02I-submission-api-cutover.md.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/reviews/WS-ARCH-001-02-plan-review.md.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/DECISIONS.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/RISKS.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-04B2-default-checker-execution.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-05A-admission-consumption-binding.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-05B-submission-api-cutover.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/ACTIVATION_CUSTODY.md.agent-loop/initiatives/WS-AUTH-001-workstream-authorization-service/CHUNK_MAP.md.agent-loop/initiatives/WS-XINT-002-art-auth-end-to-end/CHUNK_MAP.md.agent-loop/initiatives/WS-XINT-002-art-auth-end-to-end/PLAN.md.agent-loop/initiatives/WS-XINT-002-art-auth-end-to-end/chunks/WS-XINT-002-05A-initial-submission-preparation.md.agent-loop/initiatives/WS-XINT-002-art-auth-end-to-end/chunks/WS-XINT-002-05B-submission-binding-consumption.md.agent-loop/initiatives/WS-XINT-003-rev-auth-end-to-end/ACTION_CUSTODY.mddocs/architecture_checker_framework.mddocs/architecture_data_model.mddocs/architecture_lockdown.mddocs/current_system_data_flow.htmldocs/decision_0011_submission_artifact_policy_drives_pre_submit.mddocs/glossary.mddocs/operations_project_operating_manual.mddocs/spec_authorization_service.mddocs/template_checker_policy.mddocs/template_submission_artifact_policy.mddocs/template_submission_packet.md
| - [ ] One typed CHECKER API accepts immutable PROJECT/TASK lineage and returns | ||
| a deterministic immutable effective-plan contract. | ||
| - [ ] Execution results expose bounded result facts without ART scratch or | ||
| provider details. | ||
| - [ ] ART remains the sole owner of durable evidence identity/persistence, pass | ||
| capability, and admission attachment; CHECKERS creates no parallel | ||
| evidence aggregate. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 \
'storage_scheme|PreSubmissionExecutionCustody|PreSubmissionExecutionResult' \
backend/app/modules/checkers backend/app/modules/artifactsRepository: Flow-Research/workstream
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- reviewed chunk ---'
sed -n '1,120p' .agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02C-checker-pre-submit-api.md
printf '%s\n' '--- candidate checker API files ---'
fd -i -t f 'api' backend/app/modules/checkers backend/app/modules/artifacts | sort
printf '%s\n' '--- checker API references and execution-result imports ---'
rg -n -C 4 \
'checkers\.api|from app\.modules\.checkers\.api|PreSubmissionExecutionResult|PreSubmissionExecutionCustody|custody' \
backend/app/modules/checkers backend/app/modules/artifacts backend/tests tests 2>/dev/null | head -n 1200
printf '%s\n' '--- relevant tests by name ---'
fd -i -t f . backend/tests tests 2>/dev/null \
| rg -i 'checker|pre.?submit|evidence|boundary|api' \
| sort | head -n 300Repository: Flow-Research/workstream
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository state ---'
git status --short
git diff --stat
printf '%s\n' '--- checker module files ---'
git ls-files backend/app/modules/checkers | sort
printf '%s\n' '--- execution result definitions ---'
sed -n '1,190p' backend/app/modules/checkers/pre_submit_execution.py
sed -n '540,590p' backend/app/modules/checkers/pre_submit_execution.py
printf '%s\n' '--- public API/export declarations ---'
rg -n -C 3 \
'__all__|TypeAlias|Protocol|class .*API|def .*api|PreSubmissionExecutionResult|PreSubmissionEntryResult|EffectivePreSubmissionExecutionPlan' \
backend/app/modules/checkers backend/tests/architecture/test_module_boundaries.py
printf '%s\n' '--- boundary and contract assertions ---'
rg -n -C 5 \
'private|checkers|storage_scheme|custody|provider|result|api' \
backend/tests/architecture/test_module_boundaries.py \
backend/tests/test_default_pre_submit_execution.py \
backend/tests/test_submission_bundle_admission.py \
backend/tests/test_effective_pre_submit_execution.py \
2>/dev/null | head -n 1600
printf '%s\n' '--- AST field/import facts ---'
python3 - <<'PY'
import ast
from pathlib import Path
paths = [
Path("backend/app/modules/checkers/pre_submit_execution.py"),
Path("backend/app/modules/artifacts/submission_materialization.py"),
Path("backend/app/modules/artifacts/pre_submit_evidence.py"),
]
for path in paths:
tree = ast.parse(path.read_text())
print(f"{path}:")
for node in ast.walk(tree):
if isinstance(node, ast.ClassDef) and node.name in {
"PreSubmissionExecutionCustody",
"PreSubmissionExecutionResult",
}:
fields = []
for item in node.body:
if isinstance(item, ast.AnnAssign) and isinstance(item.target, ast.Name):
fields.append(item.target.id)
print(f" class {node.name} fields={fields}")
imports = []
for node in tree.body:
if isinstance(node, ast.ImportFrom):
imports.append((node.module, [a.name for a in node.names]))
for module, names in imports:
if module and "checkers" in module:
print(f" import {module}: {names}")
PYRepository: Flow-Research/workstream
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository state ---'
git status --short
git diff --stat
printf '%s\n' '--- checker module files ---'
git ls-files backend/app/modules/checkers | sort
printf '%s\n' '--- execution result definitions ---'
sed -n '1,190p' backend/app/modules/checkers/pre_submit_execution.py
sed -n '540,590p' backend/app/modules/checkers/pre_submit_execution.py
printf '%s\n' '--- public API/export declarations ---'
rg -n -C 3 \
'__all__|TypeAlias|Protocol|class .*API|def .*api|PreSubmissionExecutionResult|PreSubmissionEntryResult|EffectivePreSubmissionExecutionPlan' \
backend/app/modules/checkers backend/tests/architecture/test_module_boundaries.py
printf '%s\n' '--- boundary and contract assertions ---'
rg -n -C 5 \
'private|checkers|storage_scheme|custody|provider|result|api' \
backend/tests/architecture/test_module_boundaries.py \
backend/tests/test_default_pre_submit_execution.py \
backend/tests/test_submission_bundle_admission.py \
backend/tests/test_effective_pre_submit_execution.py \
2>/dev/null | head -n 1600
printf '%s\n' '--- AST field/import facts ---'
python3 - <<'PY'
import ast
from pathlib import Path
paths = [
Path("backend/app/modules/checkers/pre_submit_execution.py"),
Path("backend/app/modules/artifacts/submission_materialization.py"),
Path("backend/app/modules/artifacts/pre_submit_evidence.py"),
]
for path in paths:
tree = ast.parse(path.read_text())
print(f"{path}:")
for node in ast.walk(tree):
if isinstance(node, ast.ClassDef) and node.name in {
"PreSubmissionExecutionCustody",
"PreSubmissionExecutionResult",
}:
fields = []
for item in node.body:
if isinstance(item, ast.AnnAssign) and isinstance(item.target, ast.Name):
fields.append(item.target.id)
print(f" class {node.name} fields={fields}")
for node in tree.body:
if isinstance(node, ast.ImportFrom) and node.module and "checkers" in node.module:
print(f" import {node.module}: {[a.name for a in node.names]}")
PYRepository: Flow-Research/workstream
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
checker = Path("backend/app/modules/checkers/pre_submit_execution.py")
materialization = Path("backend/app/modules/artifacts/submission_materialization.py")
evidence = Path("backend/app/modules/artifacts/pre_submit_evidence.py")
api_files = sorted(Path("backend/app/modules/checkers").glob("api/**/*.py"))
def fields(path, class_name):
tree = ast.parse(path.read_text(encoding="utf-8"))
for node in tree.body:
if isinstance(node, ast.ClassDef) and node.name == class_name:
return [
item.target.id
for item in node.body
if isinstance(item, ast.AnnAssign)
and isinstance(item.target, ast.Name)
]
return None
def matching_attributes(path, names):
tree = ast.parse(path.read_text(encoding="utf-8"))
hits = []
for node in ast.walk(tree):
if isinstance(node, ast.Attribute) and node.attr in names:
hits.append(node.attr)
return sorted(set(hits))
print("api_files=", [str(path) for path in api_files])
print("PreSubmissionExecutionCustody.fields=", fields(checker, "PreSubmissionExecutionCustody"))
print("PreSubmissionExecutionResult.fields=", fields(checker, "PreSubmissionExecutionResult"))
print("submission_materialization.custody_accesses=",
matching_attributes(materialization, {"custody", "storage_scheme"}))
print("pre_submit_evidence.custody_accesses=",
matching_attributes(evidence, {"custody", "storage_scheme"}))
PY
printf '%s\n' '--- ART persistence custody path ---'
sed -n '536,610p' backend/app/modules/artifacts/pre_submit_evidence.pyRepository: Flow-Research/workstream
Length of output: 4586
Separate the public CHECKER result from ART custody.
When 02C defines checkers.api, do not expose PreSubmissionExecutionResult.custody. PreSubmissionExecutionCustody contains storage_scheme and other ART custody fields that ART persistence reads. Define a separate immutable public result with only bounded checker facts. Keep custody fields in the ART-owned adapter. Add a contract test that rejects these fields from the public CHECKER result.
🤖 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-ARCH-001-modular-monolith-boundaries/chunks/WS-ARCH-001-02C-checker-pre-submit-api.md
around lines 64 - 70, Update the 02C CHECKER API contract so its immutable
public execution result excludes PreSubmissionExecutionResult.custody and all
PreSubmissionExecutionCustody fields, including storage_scheme. Keep custody
data in the ART-owned adapter, expose only bounded checker facts publicly, and
add a contract test that rejects custody fields from the public CHECKER result.
Intent
Replace the obsolete combined ART/XINT 05-wave planning with ownership-correct,
PR-sized contracts before any submission capability is activated.
What changed
AUTH, composition, and the final public clean cut.
proof belongs to AUTH activation chunks 02G and 02H.
paths all use verified admissions and post-submit checker/REV prerequisites
are live.
custody, ART handoff, XINT revision custody, specifications, templates, and
operating documentation.
app.interfaces.artifact_operationstypes tocanonical owner APIs and reuse of ART's existing binding/evidence patterns.
Scope
Planning and documentation only. No backend runtime, schema, migration,
authorization availability, route, checker, review, or contribution behavior
changes.
Evidence
git diff --checkpython3 scripts/check_markdown_links.pypython3 scripts/check_stale_workstream_wording.pypython3 scripts/check_stale_artifact_contracts.pypython3 scripts/check_stale_authorization_docs.pyAll passed locally.
Internal review
All valid findings were incorporated. The detailed review record is
.agent-loop/initiatives/WS-ARCH-001-modular-monolith-boundaries/reviews/WS-ARCH-001-02-plan-review.md.Human review focus
TASKS.
checker-output/repair visibility, and REV admission are ready.
Merge ownership
Human maintainers retain merge authority. This PR does not authorize automatic
implementation of 02A or any later chunk.
Summary by CodeRabbit