Skip to content

Expose tracked insertion and field safety facts - #64

Closed
pedroassumpcao wants to merge 7 commits into
tensorbee:mainfrom
Symbolic-ai:pedro/sym-5087_tracked_insertion_field_facts
Closed

Expose tracked insertion and field safety facts#64
pedroassumpcao wants to merge 7 commits into
tensorbee:mainfrom
Symbolic-ai:pedro/sym-5087_tracked_insertion_field_facts

Conversation

@pedroassumpcao

@pedroassumpcao pedroassumpcao commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve tracked insertion content as an ordered paragraph reader projection
  • expose the insertion projection through RevisionRef without restoring a compatibility facade
  • report unsupported semantic attributes retained by simple and complex field sources

Review follow-up

  • rebased onto current main
  • preserve the existing direct-run and nested-revision projections while adding the paragraph view
  • add regression coverage for nested insertion and deletion wrappers at direct-run boundaries
  • ready for maintainer F-ID integration

Validation

  • cargo test -p rdocx-oxml --lib
  • focused rdocx insertion and field reader tests
  • cargo clippy -p rdocx-oxml -p rdocx-layout -p rdocx --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • python3 scripts/hash_harness.py --check

The local unfiltered rdocx run only reaches the existing environment-dependent word_and_powerpoint_chart_pixels_are_identical rasterizer oracle assertion. The failure does not exercise these reader changes.

@mantissaman mantissaman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes for a confirmed revision regression.\n\n- : insertion parsing now stores an empty list and clones only direct paragraph runs into . Existing consumers of therefore lose nested deletion and insertion wrappers. CI confirms this in , where "D" disappears, and , where "nested" becomes empty. Preserve the nested revision projection used by existing layout consumers while adding the paragraph view.\n\nTest and MSRV both fail on this regression. This PR is also a draft, is based on the S60 main SHA, and has not gone through the F-ID sprint integration workflow.

@mantissaman
mantissaman dismissed their stale review August 31, 2026 16:54

Superseded immediately because shell quoting stripped technical identifiers from the body.

@mantissaman mantissaman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes for a confirmed revision regression.

  • crates/rdocx-oxml/src/revision.rs:119: insertion parsing now stores an empty "nested_revisions" list and clones only direct paragraph runs into "RevisionContent". Existing consumers of "RevisionContent" therefore lose nested deletion and insertion wrappers. CI confirms this in "revision_views_project_wrapped_runs_in_document_order", where "D" disappears, and "nested_only_revision_wrappers_project_their_visible_runs", where "nested" becomes empty. Preserve the nested revision projection used by existing layout consumers while adding the paragraph view.

Test and MSRV both fail on this regression. This PR is also a draft, is based on the S60 main SHA, and has not gone through the F-ID sprint integration workflow.

@pedroassumpcao
pedroassumpcao force-pushed the pedro/sym-5087_tracked_insertion_field_facts branch from 6d4845a to e244cb9 Compare August 31, 2026 17:54
@pedroassumpcao
pedroassumpcao marked this pull request as ready for review August 31, 2026 18:12
@pedroassumpcao

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in commit e244cb9: nested insertion revisions are preserved instead of being flattened or dropped, with regression coverage for nested tracked insertions. The branch is ready for maintainer F-ID integration. Test, MSRV, Clippy, formatting, bindings, WASM, Word fidelity, and release-regression checks pass. Package oxml-layout remains red only because of the current-main font-inventory workflow mismatch.

@mantissaman mantissaman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-review complete. The original parse-content projection and nested-revision facts are restored, the regression covers direct and nested revision content, and the substantive CI gate passes. The remaining oxml-layout package-inventory failure is inherited unchanged from main and unrelated to this diff. Ready for maintainer integration.

@mantissaman

Copy link
Copy Markdown
Contributor

Thank you, @pedroassumpcao. We reviewed the updated 5cb5cba head and adopted the tracked insertion, nested revision, and field safety facts through F-X071. The contribution was integrated with the sprint's bounded revision-depth and namespace-preservation hardening, then passed the complete workspace gate. The result landed on main in merge commit 56bcdc1 and tag s62, with contributor credit recorded in the delivery ledger. We are closing this PR as integrated through the maintainer sprint workflow rather than merging its branch directly.

@mantissaman mantissaman closed this Sep 1, 2026
@mantissaman

Copy link
Copy Markdown
Contributor

Thank you @pedroassumpcao. v0.12.0 includes bounded nested-revision projection, preserved insertion facts, and ordered field-display safety from this proposal through a reviewed hardened equivalent.

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