feat!: collapse FileMeta into a single type - #626
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5abc4472a7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
andiwand
force-pushed
the
feat/logger-value-type
branch
from
July 27, 2026 19:26
46e03ee to
5ce4a5d
Compare
andiwand
force-pushed
the
feat/flatten-file-meta
branch
from
July 27, 2026 19:27
2ee7a93 to
22a35ad
Compare
andiwand
force-pushed
the
feat/logger-value-type
branch
from
July 27, 2026 19:34
5ce4a5d to
87f7cf3
Compare
andiwand
force-pushed
the
feat/flatten-file-meta
branch
from
July 27, 2026 19:35
22a35ad to
5c4fc64
Compare
andiwand
added a commit
to opendocument-app/OpenDocument.test.output
that referenced
this pull request
Jul 27, 2026
`odr::FileMeta` no longer carries an optional nested `DocumentMeta`, so "is a document" is now signalled by `document_type != unknown`. The backends previously materialised a default-constructed `DocumentMeta` before returning, which made an encrypted OOXML package report `"documentType": "unknown"` — a value that said nothing the neighbouring `fileType: unnamed` / `fileCategory: unknown` / `isEncrypted: true` did not already say, and that a plain text file never got in the first place. These two files are the only outputs affected (326 of 328 matched). See opendocument-app/OpenDocument.core#626.
`FileMeta` carried an `std::optional<DocumentMeta>`, so every reader of a
document property went through two hops and an optional check, and every
backend building one had to remember to materialise the nested struct
first. The indirection bought nothing: `DocumentMeta` was never handed out
independently except through `DocumentFile::document_meta()`, which is
just a second way of reading what `file_meta()` already returns.
`DocumentMeta`'s fields move up into `FileMeta` and the optional's
presence check becomes `document_type != DocumentType::unknown`.
`FileMeta` is now a plain aggregate — the backends that spelled
`{file_type(), mimetype(), false, std::nullopt}` use designated
initializers instead.
The JSON from `meta_to_json` is unchanged: it already flattened both
structs into one object.
BREAKING CHANGE:
- `odr::DocumentMeta` is gone; its fields are members of `odr::FileMeta`.
- `FileMeta::document_meta` is gone. `document_type` is now a direct
member, `unknown` when the file is not a document; the remaining
document fields are only meaningful when it is set.
- `DocumentFile::document_meta()` and the
`abstract::DocumentFile::document_meta()` virtual are gone — use
`file_meta()`.
- `FileMeta`'s two constructors are gone; it is an aggregate.
- Java: `DocumentMeta` is gone, its fields are on `FileMeta`, and
`DocumentFile.documentMeta()` is removed.
- Python: `DocumentMeta` is gone, its fields are on `FileMeta`, and
`DocumentFile.document_meta()` is removed.
The PDF backend keeps its all-or-nothing metadata semantics: a malformed
structure leaves every document field unset rather than half-filled.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbnNCsz6rkVKHW2MWL47m5
GCC rejects the designated initializers that name only the first three members under `-Werror=missing-field-initializers`, even though the rest carry default member initializers. Build the value field by field instead, matching how `odf_meta` / `ooxml_meta` / `oldms_file` already do it. Also spell out why the JSON document block is gated on `document_type`: an office container we cannot name a type for (an encrypted OOXML package, an ODF file with an unrecognized mimetype) now omits those keys instead of reporting `"documentType": "unknown"`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbnNCsz6rkVKHW2MWL47m5
An encrypted OOXML package no longer reports `"documentType": "unknown"`, since `document_type != unknown` is now the "is a document" signal and the backends no longer materialise a default `DocumentMeta` on the way out. The two affected outputs are the only ones that changed (326 of 328 matched); no HTML moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbnNCsz6rkVKHW2MWL47m5
andiwand
force-pushed
the
feat/flatten-file-meta
branch
from
July 27, 2026 19:50
5c4fc64 to
6300a72
Compare
andiwand
enabled auto-merge (squash)
July 27, 2026 19:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Clears the second of the two
Breaking API changes (next major)items indocs/design/README.md(the first landed in #625). With this the section is emptyand both entries are removed, so nothing deprecated or explicitly deferred remains
before the v6 tag.
Why
FileMetacarried anstd::optional<DocumentMeta>, so every reader of a documentproperty went through two hops and an optional check, and every backend building
one had to remember to materialise the nested struct first. The indirection bought
nothing —
DocumentMetawas never handed out independently except throughDocumentFile::document_meta(), which is just a second way of reading whatfile_meta()already returns.What
DocumentMeta's fields move up intoFileMeta; the optional's presence checkbecomes
document_type != DocumentType::unknown.Breaking changes
C++
odr::DocumentMetais gone; its fields are members ofodr::FileMeta.FileMeta::document_metais gone.document_typeis a direct member,unknownwhen the file is not a document; the remaining document fields are only
meaningful when it is set.
DocumentFile::document_meta()and theabstract::DocumentFile::document_meta()virtual are gone — use
file_meta().FileMeta's two constructors are gone; it is an aggregate.Java —
DocumentMetaremoved, its fields are onFileMeta,DocumentFile.documentMeta()removed.Python —
DocumentMetaremoved, its fields are onFileMeta,DocumentFile.document_meta()removed.One JSON change (an earlier revision of this description wrongly said "unchanged")
meta_to_jsonis unchanged for every file with a known document type — it alreadyflattened both structs into one object.
It does change for an office container whose document type cannot be named.
The old backends assigned a default-constructed
DocumentMetabefore returning,so an encrypted OOXML package emitted
"documentType": "unknown". The flat structcannot distinguish "not a document" from "document of unknown type", so those files
now omit the document keys like any other non-document.
The reference-output run puts an exact number on the blast radius: 326 of 328
outputs matched, and the two that changed are
docx/encrypted.docx/meta.jsonandxlsx/encrypted.xlsx/meta.json, each losing one line:{ - "documentType": "unknown", "fileCategory": "unknown", "fileType": "unnamed", "isEncrypted": true }That line said nothing its three neighbours did not already say, and a plain text
file never got a
documentTypekey at all — so the new behaviour is also the moreconsistent one. Reference outputs updated accordingly (no HTML moved, and
odr-privateis untouched).Thanks @chatgpt-codex-connector for catching this — it was a real behaviour change
hiding behind an "output unchanged" claim. The gate is now commented in
odr_meta_util.cppso it reads as deliberate.Note on the PDF backend
populate_document_metawas all-or-nothing: a parse throw leftdocument_metaunset rather than half-filled. Flattening would have silently lost that, since
fields are now written straight onto
FileMeta, so the helper fills a copy andcommits it only on success. Behaviour is preserved and the invariant is now
spelled out in
pdf/AGENTS.md.Note on aggregate initialisation
The backends first used designated initializers naming only the three non-document
members. GCC rejects that under
-Werror=missing-field-initializerseven thoughthe remaining members have default member initializers, so they build the value
field by field instead — matching how
odf_meta/ooxml_meta/oldms_filealready did it.
Test plan
matched before the reference bump, the 2 above after.
reports
document_type == unknownwith the document fields unset — the sentinelthat replaces the optional.
FileMetaconstructor signature.