Skip to content

Detect ingest content class and label every object write - #452

Merged
fazpu merged 13 commits into
mainfrom
fix/content-type-detection-and-storage-class
Sep 29, 2026
Merged

fazpu merged 13 commits into
mainfrom
fix/content-type-detection-and-storage-class

Conversation

@fazpu

@fazpu fazpu commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Problem

Uploader-declared MIME could route contradictory bytes before E0 storage, and object writes omitted storage-class metadata.

Scope

D132 byte-class admission before raw/catalog writes; typed HTTP refusals; explicit hot/cold class on every object-store write. Merged current main and reconciled D133/D138 family routing and D139's PDF classification rule. No new format-registry implementation.

Important decisions

Recognized PDF, media, Office, Parquet, and Photoshop bytes determine the stored class even with a misleading filename. Compatible registry hints preserve text-bearing, archive, and dataset families; converters validate complete structures. Unknown binary follows D138's binary-card path. A recognized PDF always stores as application/pdf; this PR adds no text-layer shortcut. Content-hash rows retain their first MIME except D117's existing parked-route correction. Historical wrong routable classes need a separate migration and reprocessing plan. Required storage labels are compatible with UMC D82's gateway, which accepts labelled writes.

Sources inspected

CLAUDE.md, plan/README.md, design-corpus skill, decisions D38/D51/D55/D117/D132/D133/D138/D139, E0 and format-conversion designs, registry and E0 implementations, object-store adapters, managed text metering, CI workflow and tests.

Validation / review

Head 2e2867e1: make lint passed; uv run ruff format --check src/ benchmarks/ passed (614 files); make typecheck passed (0 errors, 0 warnings); python3 .github/ci/check_test_inventory.py passed (137 unit, 69 integration, 206 discovered); uv run lint-imports passed (5 contracts); python scripts/generate_docs_llms.py --check and python scripts/check_docs_truth.py passed. Focused detector/metering/registry/routing tests: 167 passed. make test started on checkpoint 3469e6f7 and passed (2,822 passed, 916 skipped, 1 warning); final-head CI packs are the integration authority.

GitHub CI on this head passed: Unit 2,647 passed/7 skipped; Contract smoke 141 passed; adapters 679 passed/4 skipped; surfaces 1,146 passed/2 skipped; workers 826 passed. Quality, Compose quickstart, engine image, docs builds, client compatibility, PR gate, and CLA also passed. The docs deploy job was skipped by its workflow.

Cursor rounds one and two both returned REQUEST_CHANGES; their verbatim output and disposition tables are in design/reviews/REVIEW_cursor_pr452_merge_2026-09-29.md and design/reviews/REVIEW_cursor_pr452_merge_round2_2026-09-29.md. Adopted findings and the later CI fixture fixes are pushed. The task caps Cursor at two rounds; the PR comment links both records.

Limitations

Signature checks establish a class; converters validate full structure. The stock PDF text-layer converter and oversized-PDF behavior already on main are outside this PR; D139's every-page OCR implementation remains separate. Local PostgreSQL 17 lacks pg_textsearch, so migration-backed suites could not complete locally (contract attempt: 123 passed, 15 failed, 3 errors at extension creation). The throwaway cluster was stopped and removed; CI passed with the repository's PostgreSQL 19 image.

Follow-up work

Lead engineer handles final review and merge. Historical MIME correction and D139 converter implementation remain separate tracks.

Contributor agreement

Signing on behalf of a legal entity (leave blank if accepting individually):

@fazpu

fazpu commented Sep 22, 2026

Copy link
Copy Markdown
Member Author

Cursor review round 1 (detached worktree, PR #452)

BOM text, CRLF text, and a multi-megabyte PDF are accepted, and this change adds no dependency. Every production write_bytes call passes hot or cold. Conversion, stored MIME, original storage class, and managed metering then follow whatever MIME detection returns, so the defects below become the route, the catalog type, the storage tier, and the bill.

  1. BLOCKER — src/rememberstack/core/content_detection.py:59
    Failure: %PDF- anywhere in the first 1024 bytes is classified as application/pdf before PNG, JPEG, ZIP, ftyp, or UTF-8 text. A Markdown note whose first kilobyte contains that token is refused with content_type_mismatch when declared text/markdown. The same token inside a PNG comment, or a ZIP member name, stores the file as application/pdf when the client sends application/octet-stream (the SDK default). That selects the PDF route and a cold original instead of hot image or office. An ASCII PDF whose header starts at offset 1024 is the other way around: text/plain, the passthrough route, and the doc-text rate (classify_doc_text accepts it).
    Fix: Apply offset-0 signatures first. Treat %PDF- in the first 1024 bytes as PDF only when those leading bytes are not valid text and not another signature. Add tests for text that mentions %PDF-, a PNG/ZIP with that token in the first kilobyte, and an ASCII PDF header at offset 1024.

  2. BLOCKER — src/rememberstack/core/text_metering.py:76
    Failure: Managed exclusions for HTML, RTF, SVG, XML, and JVBERi0 run on the raw prefix. A UTF-8 BOM is not stripped there. b"\xef\xbb\xbf<html>…" and b"\xef\xbb\xbfJVBERi0…" are measured as text/plain (counts 28 and 12). The same HTML without the BOM raises rate_class_ambiguous. Detection has already normalized the MIME to text, so E0 admits it and bills the doc-text rate. D132 says those exclusions still apply after class detection.
    Fix: Drop a leading UTF-8 BOM before the structured-text and JVBERi0 checks. Test BOM-prefixed HTML, RTF, SVG, and JVBERi0.

  3. HIGH — src/rememberstack/core/content_detection.py:68
    Failure: BM (2 bytes) and ID3 (3 bytes) win over text. BM25 ranking declared text/plain is content_type_mismatch. Declared application/octet-stream, it is stored as image/bmp and routed hot. ID3 tags are metadata is refused the same way. The MPEG frame test at line 85 is also only a few bits: b"\xff\xfb\x90\x00\x00\x01" and UTF-16 LE b"\xff\xfeA\x00" become audio/mpeg (hot) instead of unsupported_binary_content.
    Fix: Require a real BMP header (reserved zero fields and a plausible size), a real ID3v2 header, and a non-reserved MPEG frame. Reject UTF-16 as binary. Test these inputs for both text/plain and application/octet-stream.

  4. HIGH — src/rememberstack/core/content_detection.py:83
    Failure: Any ISO BMFF file whose bytes 4:8 are ftyp becomes video/mp4. An M4A file declared audio/mp4 and a HEIC file declared image/heic are content_type_mismatch. With application/octet-stream they are stored as video/mp4, so storage stays hot but the conversion route is video.
    Fix: Read the major brand and map audio brands (M4A , M4B ) and image brands (heic, avif, mif1) to those classes. On a class mismatch, return 422. Test M4A and HEIC with both a typed declaration and application/octet-stream.

  5. MEDIUM — src/rememberstack/core/content_detection.py:132
    Failure: Image subtypes must match exactly. A JPEG declared image/jpg (a common client type for the same bytes) raises content_type_mismatch and is not stored. Audio subtypes do not have this check; bytes win and the file is accepted.
    Fix: Normalize aliases such as image/jpg and image/pjpeg to image/jpeg before the subtype comparison. Test that alias against a JPEG signature.

VERDICT: REQUEST_CHANGES

Dispositions

Finding Disposition Change
1 PDF precedence Adopted Binary signatures precede shifted-PDF inspection; shifted PDF needs object and EOF markers. Regression tests cover text, PNG, office ZIP, and an offset-1024 ASCII PDF.
2 BOM metering Adopted Strip a UTF-8 BOM before structured and armoured text exclusions; regression tests cover HTML, RTF, SVG, XML, and JVBERi0.
3 weak media prefixes Adopted Validate BMP, ID3, FLAC, Ogg, AAC, and MPEG frame header fields and lengths; test printable prefixes, short frames, UTF-16, and complete frames.
4 BMFF classes Adopted Classify M4A/M4B as audio and HEIC/AVIF brands as images; test typed and generic declarations.
5 JPEG aliases Adopted Normalize image/jpg and image/pjpeg before exact image subtype comparison.

The substantive fixes were pushed as e111ec98; Cursor round 2 is running on that head.

@fazpu

fazpu commented Sep 22, 2026

Copy link
Copy Markdown
Member Author

Cursor review round 2 (detached worktree, PR #452, head e111ec9)

The first-round signature fixes hold for printable BM and ID3, short MPEG syncs, JPEG aliases, a single leading UTF-8 BOM, and a %PDF- file of any size. Four holes still let crafted or ordinary files take the wrong class.

  1. HIGH — src/rememberstack/core/content_detection.py:165 and :255; src/rememberstack/core/text_metering.py:71
    A spec-valid PDF whose header is not at byte 0 is classified as text/plain when %%EOF sits more than 2048 bytes from the end. _has_shifted_pdf_header requires the marker in content[-2048:], then _detected_mime falls through to UTF-8. Metering only treats %PDF- as binary when it is the first byte, so it does not catch the miss.
    Failure: b"\n%PDF-1.4\n1 0 obj\n<<>>\nendobj\n%%EOF\n" + b" " * 2049, declared text/plain or application/octet-stream, is stored as text/plain, routed through the passthrough converter, kept on the cold text path, and on a managed deployment measured as doc-text. The same happens when the header starts at byte 1025 or later.
    Fix: once %PDF- is inside the preamble window and an n n obj token follows, accept %%EOF anywhere after the header. If those tokens exist and the shifted check still fails, do not return text/plain. After stripping leading BOMs and whitespace, classify_doc_text should apply the same PDF tokens before the doc-text rate. Add a regression for a newline-prefixed PDF with trailing padding.

  2. HIGH — src/rememberstack/core/text_metering.py:76
    Structured-text exclusions strip one leading UTF-8 BOM and then ASCII whitespace, so a second BOM, or a BOM after whitespace, still hides the payload.
    Failure: b"\r\n\xef\xbb\xbf<html><p>bill me</p></html>" or b"\xef\xbb\xbf\xef\xbb\xbf{\\rtf1 ...}", declared text/plain, is detected as text and classify_doc_text returns a doc-text measurement. RTF, XML, SVG, and JVBERi0 armour fail the same way. A single leading BOM is covered and still rejected.
    Fix: loop, removing a UTF-8 BOM and leading whitespace until the bytes stop changing, and run the structured and armoured checks on what remains. Test a BOM after CRLF and a doubled BOM for every excluded prefix.

  3. HIGH — src/rememberstack/core/content_detection.py:59
    GIF87a and GIF89a are printable, and any non-zero little-endian screen size counts. Ordinary ASCII and CRLF satisfy that, which contradicts the design rule that a printable prefix is not a binary class. BM and ID3 no longer have this hole.
    Failure: b"GIF89a\r\nhello there" declared text/plain raises content_type_mismatch and is not ingested. Declared application/octet-stream, it is stored as image/gif, routed as an image, and written hot.
    Fix: require a non-text logical-screen byte or the GIF trailer 0x3B before returning image/gif. Keep GIF89a is a format and GIF89a\r\n... on the text path.

  4. MEDIUM — src/rememberstack/core/content_detection.py:155 and :293
    ISO BMFF uses only the major brand, and every other brand becomes video/mp4. Image subtypes must match exactly, and image/heif is not an alias of image/heic.
    Failure: an AAC .m4a whose major brand is isom or mp42 (typical ffmpeg or Android output), declared audio/mp4 or audio/x-m4a, raises content_type_mismatch even when a compatible brand is M4A . An iPhone HEIC declared image/heif does the same against detected image/heic. Major brands M4A , heic, and avif themselves are handled. image/jpg and image/pjpeg are canonicalized before the JPEG subtype check.
    Fix: map a compatible M4A , M4B , or M4P brand to audio/mp4, and treat image/heif as image/heic the same way as the JPEG aliases. Add those two fixtures next to the existing major-brand tests.

Object writes, dependencies, and the cases that already match the new tests do not add another defect. Every write_bytes under src/rememberstack passes storage_class; local and MinIO adapters reject anything other than hot or cold. The MinIO CI smoke write passes cold, and src/tests/core/test_content_detection.py is on the unit-path list. The diff adds no package. A leading BOM plus CRLF Markdown stays text/markdown, and a large file that starts with %PDF- stays application/pdf.

VERDICT: REQUEST_CHANGES

Dispositions

Finding Disposition Change in 0a12edc
1 shifted PDF tail Adopted Search a bounded preamble for PDF header and object body, accept EOF anywhere, and refuse incomplete PDF bodies instead of calling them text; metering also rejects the PDF body signature.
2 repeated or offset BOM Adopted Iteratively strip whitespace and UTF-8 BOMs before structured and armoured text exclusions; test every excluded prefix under CRLF+BOM and doubled BOM.
3 printable GIF prefix Adopted Require non-text logical-screen bytes or a plausible image/extension block with trailer; test ordinary GIF-prefixed text and a binary GIF header.
4 BMFF compatible brands Adopted Read compatible brands, normalize audio/x-m4a and image/heif, and test generic major brands with M4A compatibility.

The two permitted Cursor rounds are complete. These final adopted fixes were validated locally and pushed after round 2; the lead engineer performs the final review.

@fazpu

fazpu commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

Cursor review record for the merged PR head:

Adopted the byte/family, PDF predicate, metering, typed-error documentation, and fixture findings. D117's parked-route MIME correction stays in place and is now explicit in D132. The stock PDF converter and managed PDF billing are separate D139/product work; this PR does not add a text-layer route. The second-round fixes were pushed after the two-round Cursor cap and are being checked by CI on 2e2867e1.

@cursor

cursor Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@fazpu
fazpu marked this pull request as ready for review September 29, 2026 11:25
@fazpu
fazpu merged commit adea831 into main Sep 29, 2026
19 checks passed
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.

1 participant