Skip to content

feat(ingest): each embedded docx image names its part, digest and size (#732, slice 2) - #961

Closed
cdeust wants to merge 6 commits into
mainfrom
fix/732-docx-image-digest
Closed

cdeust wants to merge 6 commits into
mainfrom
fix/732-docx-image-digest

Conversation

@cdeust

@cdeust cdeust commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

Refs #732 (tranche 2 ; l'issue reste ouverte).

Cause racine.
La tranche 1 (#893) inventoriait chaque image embarquée avec son id de relation docx (r:embed/r:id), sans résoudre cet id vers la part du paquet qui porte les octets. Rien ne permettait donc d'identifier les octets d'une image, prérequis de leur conservation (critère « bytes, digest, position and document version »).

  • core/docx_relationships.py (pur) : word/_rels/document.xml.rels → {id: part} pour chaque relation interne, cible résolue selon ECMA-376 Part 2 Annex A.3 (relative à word/, absolue depuis la racine, segments ./..). Les cibles TargetMode="External" et vides sont absentes. XML malformé → DocumentParseError.
  • infrastructure/docx_media_reader.py : lit la part de relations (plafond MAX_DOCUMENT_BYTES d'ADR-0516, UTF-8) et passe chaque part nommée en SHA-256 par blocs de 64 KiB (mémoire bornée).
  • handlers/ingest_document_media.py : composition ; chaque attachment résolu reçoit part, digest (sha256:<hex>), size. Image liée, part absente, image Confluence : champs vides ("", "", null). state reste unsupported : les octets sont identifiés, pas conservés.
  • Description de l'outil, docs/module-inventory.md et CHANGELOG mis à jour.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would change existing behavior)
  • Refactor (no functional change; rules/coding-standards.md compliance)
  • Documentation only
  • Audit-finding closure (cite the finding ID)

Test plan

  • pytest tests_py/core/test_docx_relationships.py tests_py/infrastructure/test_docx_media_reader.py tests_py/handlers/test_ingest_document*.py + tous les tests qui référencent ingest_document/DocumentAttachment/module-inventory : 178 passed, 3 skipped.
  • pytest tests_py/core/ + tests document : 3961 passed, 162 skipped, 1 échec préexistant sur main (test_wiki_coverage_dashboard.py::test_unreadable_page_is_skipped_not_fatal, environnement root).
  • ruff check / ruff format --check (0.17.3) sur les fichiers touchés : OK. python scripts/check_craftsmanship.py : OK.

Mutations visées par les nouveaux tests : résolution .. au-dessus de la racine, cible vide ou faite de seuls segments de point, TargetMode externe vs interne, relation sans Id, élément hors espace de noms OPC, plafond exact (max_bytes égal / dépassé), lecture par blocs au-delà de 64 KiB, part vide (taille 0), CRC invalide, part absente du zip.

Audit notes

Non vérifié.

  • Gate de mutation (ADR-1122) : scripts/mutation_check.sh n'a pas pu tourner en local (téléchargement de torch refusé par le proxy de l'environnement).
  • pyright non lancé localement ; job unit complet de la CI non rejoué.

Tranches restantes de #732 (hors de cette PR).

  1. Conserver les octets (état preserved-only) avec une référence de source accessible, côté stockage.
  2. États extracted et failed explicites.
  3. OCR/vision optionnels avec fournisseurs déclarés.
  4. Propriété des références partagées à la correction/suppression.
  5. Rejeu d'une fixture illustrée publique dans Claude et Codex.

https://claude.ai/code/session_018kyZYr4VjkzmxX2THRjfx7


Generated by Claude Code

#732, slice 2)

The attachment inventory from slice 1 (#893) named a docx image only by its
relationship id. The id is now resolved through word/_rels/document.xml.rels
(ECMA-376 Part 2 §9.3, Annex A.3) to the package part holding the bytes,
and that part is streamed through SHA-256. Linked images, absent parts and
Confluence images keep part/digest/size empty; state stays unsupported
until a later slice stores the bytes. Malformed relationships fail before
any write, like the main part.

Refs #732

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYr4VjkzmxX2THRjfx7
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-10T07:46:13.344672Z e27e3d1 New commits
🔒 Security Review ✅ Completed 2026-11-10T07:48:10.372598Z e27e3d1 New commits

Security findings

Advisory findings (1)

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 285257e0ae

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

f"{path} relationships part is {info.file_size} bytes, "
f"exceeds {max_bytes}-byte cap"
)
raw = archive.read(info)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wrap relationships-member read failures in DocumentReadError

When a DOCX has a readable main document but a corrupted relationships member, archive.read(info) can raise zipfile.BadZipFile directly (reproduced with a CRC mismatch). _open() only wraps failures while opening the container, so this exception escapes ingest_document.handler, which catches only DocumentReadError and DocumentParseError, instead of returning the expected document_read_failed result. Wrap member-read failures in DocumentReadError, as the existing main-part reader does.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmé et corrigé dans 94f9948 (fixup de 285257e). archive.read(info) sur word/_rels/document.xml.rels est désormais enveloppé : BadZipFile/OSError/EOFError deviennent DocumentReadError, ce qui donne document_read_failed au handler. Nouveau test test_a_relationships_part_whose_crc_fails_is_refused : il échoue avec zipfile.BadZipFile: Bad CRC-32 avant le correctif et passe après.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 94f9948963

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +107 to +108
except (zipfile.BadZipFile, OSError, EOFError) as exc:
raise DocumentReadError(f"cannot read {part} in {path}: {exc}") from exc

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wrap decompression errors in DocumentReadError

When a DOCX has a readable main document but an invalid DEFLATE block in a referenced image, member.read() raises zlib.error, which this exception handler does not catch. Unlike the previously reported CRC mismatch, which is now covered, corrupting a compressed block reproduces an uncaught decompression exception. This escapes ingest_document.handler instead of returning document_read_failed, since that handler catches only DocumentReadError and DocumentParseError. Catch and wrap zlib.error here and in read_docx_relationships_xml, whose member-read exception handler has the same gap.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmé et corrigé dans 580a9ea (fixup de 285257e), à la racine plutôt que cas par cas : les deux lectures de membre passent maintenant par _stream_member, qui convertit en DocumentReadError tout ce que zipfile peut lever en lisant ou décompressant un membre (BadZipFile, EOFError, zlib.error, lzma.LZMAError, NotImplementedError pour une méthode non supportée, RuntimeError pour un membre chiffré, OSError). Nouveau test paramétré test_a_member_with_a_corrupt_deflate_stream_is_refused (image et part de relations) : il échoue avec zlib.error: invalid block type avant le correctif et passe après.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Codex Security Review · Automatically triggered

Here are some automated security review suggestions for this pull request.

Reviewed commit: 580a9ea646

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

with _open(path) as archive:
names = set(archive.namelist())
for part in sorted(set(parts) & names):
found[part] = _digest_member(archive, part, path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Codex Security Review · Automatically triggered

P2 Badge Security: Cap decompressed media before hashing

When a DOCX from an untrusted author is ingested, every referenced ZIP member is streamed to EOF here without a per-member or aggregate decompressed-byte limit. A 522,508-byte proof archive with one 512 MiB media member was accepted and fully hashed; multiple or LZMA members can scale the work further. Streaming bounds RAM and the batch semaphore limits concurrent ingests, but neither bounds CPU, and caller cancellation leaves the worker running. Reject oversized ZipInfo.file_size values and enforce a cumulative byte budget while streaming.

SECURITY.md reference: SECURITY.md:L5-L16

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constat confirmé par lecture du code ; décision du propriétaire nécessaire avant tout push.

Ce que j'ai tracé. ZipExtFile lit exactement ZipInfo.file_size octets décompressés (au-delà, le flux est tronqué et le CRC vérifié). La somme des file_size des parts référencées borne donc le travail de digest_docx_parts. Aujourd'hui, aucune borne ne s'applique à cette somme. La mémoire reste bornée (lecture par blocs de 64 KiB), mais le temps CPU croît avec la taille déclarée. La cause tient donc en une ligne : il faut une borne sur sum(info.file_size), vérifiée avant de lire, plus le refus d'un membre dont file_size dépasse le reste du budget.

Pourquoi je ne pousse pas. Le correctif exige une constante, et la règle du dépôt (« no source, no implementation », # source: sur chaque constante) m'interdit d'en inventer une. Je n'ai pas pu lire la limite documentée de Word : support.microsoft.com et learn.microsoft.com sont bloqués par le réseau de cet environnement.

À trancher (cdeust) :

  1. Réutiliser le plafond d'ADR-0516 (MAX_DOCUMENT_BYTES, 1 MiB) par part média. C'est sourcé, mais une capture d'écran de plus de 1 MiB resterait non identifiée, avec un état explicite à définir (failed ou champs vides).
  2. Fixer un budget cumulé par document (par exemple la limite de taille totale documentée par Microsoft pour un document Word avec images), enregistré dans un ADR comme source. Je l'implémente dès que la valeur et sa source sont posées.
  3. Accepter le risque dans le périmètre de SECURITY.md (l'outil n'ingère qu'un chemin local que l'utilisateur fournit lui-même) et clore ce fil avec cette justification.

Generated by Claude Code

@claude

claude Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

PR #961 was updated after the trigger (285257e -> 580a9ea). Push again or mention Claude to run on the latest version.

@claude

claude Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

PR #961 was updated after the trigger (94f9948 -> 580a9ea). Push again or mention Claude to run on the latest version.

cdeust commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

CI autopilot : les deux échecs Claude review (runs 42750084775 et 42750438203) ont tourné sur les anciens heads 285257e et 94f9948. Le bot l'indique lui-même (« PR #961 was updated after the trigger … -> 580a9ea ») : ces échecs ne portent pas sur le code. Sur le head courant 580a9ea, Claude review est en cours et unit en file. Aucun autre check n'est en échec. Je ne relance rien : le run du head courant remplace ces deux-là.

Reste ouvert : le fil de sécurité sur le plafond des parts média, qui attend la décision du propriétaire.


Generated by Claude Code

@claude

claude Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Claude finished @cdeust's task in 2m 13s —— View job


Review 1 of 3 of PR #961: approve, no Blockers

The posted review lists three Notes that need no action before merge:

  • Images are decompressed and hashed with no size cap.
  • An image whose part is missing from the docx, or named with different letter case, looks the same as a linked image.
  • No handler test covers a read failure in the new image code.

I could not run lint, the craftsmanship gate or the tests, because those commands need an approval this job does not have. The CI Pipeline run was still in progress when I posted.

Verdict: approve

@claude

claude Bot commented Nov 10, 2026

Copy link
Copy Markdown

Review 1 of 3: PR #961 (head 580a9ea)

Five reviews: code quality, security, performance, test coverage and documentation accuracy. I found no Blockers.

The two Codex findings are fixed. _stream_member (docx_media_reader.py:58-70) now turns every error a member read can raise into DocumentReadError, including BadZipFile, zlib.error and LZMAError. Both the CRC and corrupt-deflate cases are tested. The layers follow the rules: core and infrastructure do not import each other, and the handler is the only place that combines them. Every constant has a # source: comment. A rels header that understates the size cannot get past the 1 MiB cap, because ZipExtFile stops reading at file_size.

Notes, no action before merge

  • Image parts are inflated and hashed with no size cap (docx_media_reader.py:142-144). A deflate-bomb image costs CPU and I/O, not memory. A per-media file_size cap like the ADR-0516 one would close this in a later slice.
  • A missing internal part and a part name differing only in case (ECMA-376 Part 2 §9.1.1.1) both give empty part/digest/size, which looks the same as a linked image (ingest_document_media.py:46-48).
  • No handler test reaches document_read_failed through the new media path, and test_malformed_relationships_fail_before_any_write never asserts write.calls == [] (tests_py/handlers/test_ingest_document_attachments.py).

Not checked: I could not run lint, the craftsmanship gate or the tests here, because those commands need an approval this job does not have. The CI Pipeline run was still in progress when I posted.

Verdict: approve

…and size (#732, slice 2)

fixup! feat(ingest): each embedded docx image names its part, digest and size (#732, slice 2)

unit (run 42750861159): test_every_storage_primitive_is_an_audited_site read archive.open(name) as open-write, the first positional argument of an open on an unresolved receiver being taken as its mode. The member is opened for reading: say so with mode="r".

cdeust commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

CI autopilot : unit en échec (run 42750861159, job 129618875862) sur 580a9ea. Un seul test a échoué : tests_py/architecture/test_storage_redaction_gate.py::test_every_storage_primitive_is_an_audited_site (« found but not audited: infrastructure/docx_media_reader.py, open-write »).

  • Cause racine. Le scanner ADR-1110 (tests_py/architecture/_storage_primitives.py::_open_kind) ne résout pas le type de archive. Il lit donc le premier argument positionnel de archive.open(name) comme un mode, et un mode non littéral compte comme une écriture. Le membre n'est pourtant ouvert qu'en lecture.
  • Correctif. archive.open(name, mode="r") dans _stream_member. Le mode de lecture est explicite, sans entrée d'audit ni changement du scanner.
  • Commit poussé. 6647c68 (fixup de 285257e).
  • Reçus. En local, le test échoue sans le correctif (1 failed) et passe avec. tests_py/architecture/ + tests document : 109 passed. Ruff et le gate craftsmanship passent.
  • Non vérifié. Le job unit complet de la CI n'a pas été rejoué en local, et le gate de mutation non plus (torch bloqué par le proxy).

Generated by Claude Code

@claude

claude Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Claude finished @cdeust's task in 1m 54s —— View job


Review 2 of 3: PR #961, approve with no Blockers

The commit changes nothing at runtime, because "r" is already the default mode. It fixes a false positive in test_every_storage_primitive_is_an_audited_site, which had read the bare call as an open for writing. I worked this out by reading the scanner's code: I could not run pytest because the command needs an approval this job does not have. The unit CI job on 6647c68 will confirm the test passes.

The media-size cap thread still waits on the owner's decision.

Verdict: approve

@claude

claude Bot commented Nov 10, 2026

Copy link
Copy Markdown

Review 2 of 3: PR #961 (head 6647c68)

Review 1 (580a9ea) found no Blockers, so there was nothing to re-check. This review covers only the one commit pushed since then: 6647c68, archive.open(name) → archive.open(name, mode="r") in _stream_member (mcp_server/infrastructure/docx_media_reader.py:66). Five reviews: code quality, security, performance, test coverage and documentation. I found no Blockers.

Why the fix is sound

  • Nothing changes at runtime: "r" is already the default for ZipFile.open(name, mode='r', ...), so the call returns the same streaming ZipExtFile. Security and performance are unaffected.
  • It fixes a false positive in the audit test without suppressing it:
    • The scanner cannot tell what archive is, so it names the call ?.archive.open and treats it like Path.open, with argument 0 as the mode (tests_py/architecture/_storage_primitives.py:144-146).
    • A mode it cannot read as a constant counts as a write (:122-127), so the old bare call was flagged as open-write.
    • The scanner reads the mode= keyword first (:121), so "r" now counts as a read.
    • _AUDITED is unchanged, and no shim or fallback was added.
  • The rest of the file comes out clean. zipfile.ZipFile(p) (:51) gets its default "r" from _MODE_OPENERS (_storage_primitives.py:42).
  • _stream_member is still tested through both entry points, for CRC and corrupt-deflate failures (tests_py/infrastructure/test_docx_media_reader.py:76-119).
  • The docstrings, the CHANGELOG and docs/module-inventory.md are still accurate, and so is the commit message.

Notes, no action before merge

  • mode="r" is there only to satisfy the scanner. If someone drops it later, the test fails again, so a short comment at docx_media_reader.py:66 would help.
  • The scanner can miss a real write: x.open("f", "w") on an object it cannot identify reads "f" as the mode and passes. This was there before the PR.
  • The media-size cap thread (Codex security review) still waits on the owner's decision. It is not part of this review.

Not checked: I could not run pytest here (the command needs an approval this job does not have), so I traced the scanner by reading its code. The unit CI job on 6647c68 is the run that confirms it.

Verdict: approve

…and size (#732, slice 2)

unit (run 42755857850): the mutation gate (ADR-1122) left 12 survivors and found no test importing ingest_document_media.

- docx_relationships: the all-'..' guard was redundant (popping word/ already yields ''), so it is now 'if not named'. Id and Target are read without a default and a relationship missing either is skipped; a missing-Target test pins it.
- docx_media_reader: the tests assert that the docx path appears in read errors, and that every read asks for at most 64 KiB. mode="r" (the ZipFile.open default) and 'UTF-8' vs 'utf-8' are registered equivalents.
- ingest_document_media: the lookup has no default; tests_py/handlers/test_ingest_document_media.py covers it directly.

cdeust commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

CI autopilot : unit en échec (run 42755857850, job 129636404208) sur 6647c68 à cause du gate de mutation (ADR-1122). Les 12382 tests unitaires passaient. Le gate signalait 12 mutants survivants : 6 dans docx_relationships.py, 6 dans docx_media_reader.py. Il signalait aussi qu'aucun fichier de test n'importait ingest_document_media.

  • Cause racine. Les tests vérifiaient le comportement visible, mais pas trois propriétés :

    • qu'une relation sans Target est ignorée ;
    • que le chemin du docx figure dans les erreurs de lecture ;
    • que la lecture se fait par blocs.

    Deux mutants venaient de valeurs par défaut inutiles (rel.get("Id", ""), parts.get(ref, "")), et un autre d'une garde redondante (all(s == "..")).

  • Correctif.

    • docx_relationships : la garde devient if not named. Id et Target sont lus sans valeur par défaut, et une relation à laquelle il manque l'un ou l'autre est ignorée. Un test « relation sans Target » fige ce comportement.
    • docx_media_reader : les tests vérifient le chemin du docx dans l'erreur, et qu'aucune lecture ne demande plus de 64 KiB.
    • ingest_document_media : la recherche se fait sans valeur par défaut. Le module a son propre fichier de test, tests_py/handlers/test_ingest_document_media.py.
    • Deux équivalents sont enregistrés dans memory/mutation-equivalents.json : mode="r", qui est la valeur par défaut de ZipFile.open, et "UTF-8" contre "utf-8" (même codec).
  • Commit poussé. 0abab42 (fixup de 285257e).

  • Reçus.

    • scripts/mutation_check.sh rejoué en local avec mutmut 3.8.1 sur les trois modules : 0 survivant sur docx_relationships et ingest_document_media. Sur docx_media_reader, il reste 2 survivants, tous deux des équivalents enregistrés acceptés par le registre.
    • Avant le correctif, les 12 survivants de la CI se reproduisaient en local, à l'identique.
    • 154 tests passés (document, architecture, registre). Ruff et le gate craftsmanship passent.
  • Non vérifié. Le job unit complet de la CI n'a pas été rejoué en local (torch est bloqué par le proxy, mutmut a tourné sans).


Generated by Claude Code

cdeust commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner Author

Mise à jour (CI autopilot) : dépassé. Le conflit a été résolu par e27e3d1 (« Merge main: keep both changelog entries »), qui garde les deux entrées du CHANGELOG. J'ai vérifié : le CHANGELOG ne contient aucun marqueur de conflit, et le code de la PR est identique à celui du rebase 36ef8fd. La branche fix/732-docx-image-digest-rebased ne sert plus. Je ne la supprime pas : la routine ne supprime aucune branche.


Generated by Claude Code

@claude

claude Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Claude finished @cdeust's task in 2m 26s —— View job


Review 3 of 3: PR #961, approve with no Blockers

All of the PR is accepted, and no file or commit carries an open Blocker. The review lists three Notes that need no action before merge:

  • Targets are not percent-decoded, so an image whose target has an encoded character, such as %20, keeps empty fields.
  • The _resolve_part docstring is slightly wrong: a target like media/.. gives word, not "". This is harmless.
  • The media-size cap thread still waits on the owner's decision.

I could not run lint, the craftsmanship gate, pytest or mutmut here, so the reviewers traced the code by reading it. The unit job on e27e3d1 is the run that confirms the mutation gate passes.

Verdict: approve

@claude

claude Bot commented Nov 10, 2026

Copy link
Copy Markdown

Review 3 of 3: PR #961 (head e27e3d1)

Reviews 1 (580a9ea) and 2 (6647c68) found no Blockers, so there was nothing to re-check. This review covers only what was pushed after 6647c68:

Five reviews (code quality, security, performance, test coverage, documentation) found no Blockers.

What I checked in 0abab42

  • Guard change (docx_relationships.py:43): if not named behaves the same as the old all(s == "..") guard. _SOURCE_DIR is the single segment ("word",), so a target made only of .. segments still ends in [], which gives "". .. still never climbs above the package root (:49-50).
  • Missing Id or Target (:76-77): a relationship without either is skipped and nothing raises. The new missing-Target test covers this.
  • Handler lookup (ingest_document_media.py:47-48): parts.get(ref) with an is not None check behaves the same as the old lookup.
  • Chunk-size test (test_a_part_is_read_in_bounded_chunks): the test is sound. CPython's ZipExtFile.read never calls itself, and the local header is read through _SharedFile. So every recorded call comes from _stream_member with _CHUNK_BYTES.
  • The two mutation equivalents (mode="r", "UTF-8" vs "utf-8"): both reasons hold. Both entries have the fields scripts/mutation_equivalents.py::parse_registry requires.
  • Layers and constants: the layer rules and the # source: rule still hold.

Notes, no action before merge

  • Targets are not percent-decoded (ECMA-376 Part 2 Annex A.3), so a target like media/my%20pic.png stays unresolved, with empty fields and no error (docx_relationships.py:42).
  • A target like media/.. resolves to the folder word, not to "". This is harmless, because no zip member has that name, but the docstring says "" is returned "when nothing names a part" (docx_relationships.py:37-38).
  • The open media-size cap thread from the Codex security review still waits for the owner's decision (option 1, 2 or 3 in that thread).

What is accepted, what is open: all of the PR is accepted (commits 285257e, 94f9948, 580a9ea, 6647c68, 0abab42, and merge e27e3d1). No file or commit carries an open Blocker. The media-size cap is a Note, a choice for the owner, and can go in a later slice of #732.

Not checked: I did not run lint, the craftsmanship gate, pytest or mutmut here. The reviewers traced the code by reading it. The unit job on e27e3d1 is the run that confirms the mutation gate passes.

Verdict: approve

cdeust commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

CI autopilot : toute la CI de e27e3d1 est verte (unit avec le gate de mutation, merge, contract, regression, e2e, install et environment sur les trois OS, CodeQL, Claude review), et les trois revues Claude ont approuvé. Mais la PR est de nouveau en conflit avec main (0917c66, après #960, #964 et #966). Le seul conflit est mécanique, dans CHANGELOG.md : une nouvelle entrée en tête de « Added ».

L'autopilot ne peut pas le résoudre : le push avec lease d'un rebase est refusé par le classifieur de l'environnement, et la routine interdit git merge origin/main. Restent pour un humain : la mise à jour de la branche, et la décision sur le fil de sécurité du plafond des parts média (options 1, 2 ou 3).


Generated by Claude Code

cdeust commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

Continued in #978.


Generated by Claude Code

@cdeust cdeust closed this Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants