docs(artifacts): split complex format extractor work - #228
Conversation
|
Warning Review limit reached
Next review available in: 36 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 (14)
📝 WalkthroughWalkthroughART-03B is decomposed into dependency approval, PDF, OOXML, and image-metadata subchunks. Planning, decision, risk, status, handoff, and downstream sufficiency documents are updated to reference the new sequence and ownership boundaries. ChangesART-03B extraction planning
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 11
🤖 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/AUTH_HANDOFF.md:
- Line 20: Use one canonical spelling for all split 03B3 subchunks and replace
range notation with explicit identifiers. In
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md:20,
enumerate every 03B3B subchunk and distinguish the 03B3B1 approval gate from
extractor implementation; in CHUNK_MAP.md:31, replace 03B3B3B-03B3B3D with
03B3B3B, 03B3B3C, and 03B3B3D; in DECISIONS.md:403-407, spell out the OOXML
ownership chunks; and in
chunks/WS-ART-001-03B-guide-source-binding-materialization.md:18-19, list the
complete executable chain while separating the approval gate from implementation
chunks.
In @.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md:
- Around line 59-64: Update the dependency graph around 03B3B1 so 03B3B2,
03B3B3A, and 03B3B4 each branch directly from 03B3B1; keep 03B3B3B/C/D dependent
on 03B3B3A, and show 03B4 joining only after all required extraction chunks
complete.
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B1-parser-dependency-approval.md:
- Around line 45-57: The acceptance criteria must bind human approval to the
exact allowlist artifact. Define a durable approval record under .agent-loop
containing the approver, timestamp, allowlist digest, and approval status;
update CI to fail closed when approval is absent or the current
backend/config/guide_extractor_dependencies.json digest differs from the
recorded digest, and ensure the record is available before downstream chunks
begin.
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B2-pdf-extractor.md:
- Around line 55-63: Update the pytest verification command in the “Verification
commands” section to include backend/tests/test_artifact_architecture.py
alongside the existing test files. Keep all other verification commands
unchanged.
- Around line 9-10: Update the 03B3B2 dependency and extraction contract to
require the 03B3B1 approval gate to be merged first and to validate the PDF
package and version against its exact pinned allowlist before installation. Make
execution fail closed when the gate is absent or the dependency is not approved,
preventing the 03B3B2 contract from running independently with an unapproved
parser.
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3A-ooxml-container-security.md:
- Around line 41-43: Make the OOXML security policy explicit and testable before
any adapter runs: define the exact allowed archive-member set, entry/count
limits, compressed and decompressed-size limits, nesting/depth limits, and XML
safety constraints, then reject archives with missing or extra members and
return a deterministic result for every violation. Update the detector and
related acceptance criteria so classification is exact rather than accepting
required subsets, and ensure all bounds are enforced before parser imports
execute.
- Around line 11-14: Make 03B3B1 approval a hard predecessor for every OOXML
chunk: in
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3A-ooxml-container-security.md
lines 11-14, explicitly reference 03B3B1 as required; in
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3B-docx-extractor.md
lines 11-14,
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3C-pptx-extractor.md
lines 11-14, and
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3D-xlsx-extractor.md
lines 11-14, add the same fail-closed gate prohibiting parser
installation/import and progression until the exact pinned allowlist from
approved 03B3B1 is available.
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3B-docx-extractor.md:
- Around line 40-42: Make the format-specific acceptance criteria observable and
deterministic: in
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3B-docx-extractor.md
lines 40-42, define DOCX paragraph/table traversal, flattening, output limits,
and exact boundary outcomes; in
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3C-pptx-extractor.md
lines 40-42, specify whether 300 is accepted and 301 rejected, plus slide/notes
inclusion and ordering; in
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3D-xlsx-extractor.md
lines 40-42, specify 100 versus 101 outcomes and deterministic handling of
formulas, cached values, merged cells, and shared strings.
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B4-image-metadata-extractors.md:
- Around line 56-67: Update the Verification commands section to include
backend/tests/test_artifact_architecture.py alongside the existing targeted
tests, and add the repository’s established coverage command that collects and
enforces 90% changed-subsystem and 78% overall coverage. Keep the existing
dependency, lint, contract, link, and diff checks unchanged.
- Around line 42-49: Update the acceptance criteria for the 03B3 image metadata
extractors to define a fixed output allowlist containing only structural
metadata, explicitly excluding EXIF, PNG text chunks, and JPEG/WebP XMP, IPTC,
and other ancillary blocks. Require non-structural metadata to be discarded and
add fixtures covering these metadata types across the supported formats.
- Around line 52-54: Update the image-only acceptance criteria in
WS-ART-001-03B3B4 to explicitly inherit 03B3A’s compressed and output byte
limits, CPU and wall-clock limits, child-kill/termination behavior, and cleanup
semantics. Require tests covering these resource and termination boundaries
alongside the existing 16,384-dimension and 40-megapixel checks.
🪄 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: 71d13e98-1a20-478d-820e-6196c3211c10
📒 Files selected for processing (17)
.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/DISCOVERY.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.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-03B-guide-source-binding-materialization.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B-complex-format-extractors.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B1-parser-dependency-approval.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B2-pdf-extractor.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3A-ooxml-container-security.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3B-docx-extractor.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3C-pptx-extractor.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B3D-xlsx-extractor.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B4-image-metadata-extractors.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B4-guide-sufficiency-continuation.md
💤 Files with no reviewable changes (1)
- .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B-complex-format-extractors.md
| ## Acceptance criteria | ||
|
|
||
| - Each candidate records exact package/version, license, maintenance, | ||
| advisories, transitive graph, native-code use, malformed-input history, | ||
| network behavior, and cancellation/timeout implications. | ||
| - The allowlist schema records package name, exact version, artifact hashes, | ||
| source/index, direct/transitive status, import names, native-wheel status, | ||
| license, advisory snapshot date, and exact format scope. | ||
| - The allowlist maps each dependency to only PDF, OOXML, or image metadata. | ||
| - CI fails closed on undeclared, unpinned, hash-drifted, or wrong-format parser | ||
| dependencies. | ||
| - Human approval of the exact list is recorded before any of 03B3B2, | ||
| 03B3B3A-03B3B3D, or 03B3B4 starts. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Bind human approval to the exact allowlist artifact.
The contract requires approval before downstream chunks start, but does not define an approval record or prove that the approved bytes are the bytes later consumed. A subsequent edit to backend/config/guide_extractor_dependencies.json could therefore retain the approval while changing packages or hashes.
Require a durable approval record containing the approver, timestamp, allowlist digest, and approval status; make CI fail closed when the current allowlist digest differs or approval is absent. As per path instructions, durable engineering memory, initiative plans, chunk contracts, policies, evidence, and review logs must live under .agent-loop/.
🤖 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-03B3B1-parser-dependency-approval.md
around lines 45 - 57, The acceptance criteria must bind human approval to the
exact allowlist artifact. Define a durable approval record under .agent-loop
containing the approver, timestamp, allowlist digest, and approval status;
update CI to fail closed when approval is absent or the current
backend/config/guide_extractor_dependencies.json digest differs from the
recorded digest, and ensure the record is available before downstream chunks
begin.
Source: Coding guidelines
| Install only the approved PDF dependency and add bounded PDF text extraction on | ||
| the existing 03B3A isolated framework. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make the 03B3B1 approval gate explicit.
This only says “approved PDF dependency”; it does not require 03B3B1 to be merged or the package/version to match its exact pinned allowlist before installation. Add that prerequisite and require fail-closed behavior so this contract cannot be executed in isolation with an unapproved parser dependency.
🤖 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-03B3B2-pdf-extractor.md
around lines 9 - 10, Update the 03B3B2 dependency and extraction contract to
require the 03B3B1 approval gate to be merged first and to validate the PDF
package and version against its exact pinned allowlist before installation. Make
execution fail closed when the gate is absent or the dependency is not approved,
preventing the 03B3B2 contract from running independently with an unapproved
parser.
…to codex/ws-art-001-03b3b-split-plan
…n' into codex/ws-art-001-03b3b-split-plan
Summary
Safety boundary
Planning only. No package, lock, workflow, application runtime, migration, AUTH, Celery, or submission code changed. No parser dependency is installed.
Evidence
Human review focus
Approve the chunk boundaries and the rule that 03B3B1 must merge with an exact pinned allowlist before any parser package can be installed.
Summary by CodeRabbit