Repository navigation
fix(id): pin documented skip policy for unknown marks (#1045) - #4685
Conversation
Unknown marks are shed by design while their text survives; unknown nodes keep returning ConversionError::UnknownType. Documents the policy on prosemirror_to_markdown, clarifies the skip arm in create_marked_text, and adds a regression test covering bare, mixed known+unknown, and code+link+unknown mark combinations.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Document and test unknown-mark policy pkgs/id/src/web/markdown.rs |
The API documentation and fallback comment describe how unknown marks are handled. A regression test checks text preservation and rendering with recognized marks. |
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- developing-today/code#985: Defines canonical ordering for incoming marks, including unrecognized marks, which share the unknown-mark handling path changed here.
Merge Risk: 
1b5d7
-
The change documents and tests the existing unknown-mark skip policy. No merge-blocking risk is identified; merge after normal checks pass.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the fix area (id) and the policy being pinned (skip policy for unknown marks), which matches the changeset. It also cites issue #1045in the title. The title is a single sentence, but …Linked Issues check Issue #1045requires a documented and tested policy for unknown marks. The diff adds aprosemirror_to_markdowndoc comment that names the skip-by-design policy and distinguishes unknown marks from u…Out of Scope Changes check The diff changes only pkgs/id/src/web/markdown.rs. It adds issue-focused documentation, clarifies existing unknown-mark handling, and adds regression coverage for issue#1045. No unrelated implement…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Title check
-
Explanation
The title names the fix area (id) and the policy being pinned (skip policy for unknown marks), which matches the changeset. It also cites issue
#1045in the title. The title is a single sentence, but it does not name the behavior as a skip. Its wording of 'pin documented skip policy' is specific enough for a teammate scanning history to see the main change: unknown marks are now skipped in prosemirror_to_markdown, with tests and a doc comment added.
Full details: Linked Issues check
-
Explanation
Issue
#1045requires a documented and tested policy for unknown marks. The diff adds aprosemirror_to_markdowndoc comment that names the skip-by-design policy and distinguishes unknown marks from unknown nodes. Thecreate_marked_textcomment separatescodehandling from unknown-mark skipping. The new regression test covers a bare unknown mark, mixed known and unknown marks, andcodepluslinkplus an unknown mark. The reportedweb::markdowntest run passed with 224 tests, and the test cases preserve known-mark andcodeoutput.
Full details: Out of Scope Changes check
-
Explanation
The diff changes only
pkgs/id/src/web/markdown.rs. It adds issue-focused documentation, clarifies existing unknown-mark handling, and adds regression coverage for issue#1045. No unrelated implementation or test changes are present.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 1 files.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
CodeRabbit Verify
- Run CodeRabbit Verify to test the changes in this pull request.
Follow-up to #1045 triage (task 6632).
Unknown marks are now an explicitly documented skip-by-design policy on
prosemirror_to_markdown(text survives, unknown styling is shed), distinct from unknown nodes which keep returningConversionError::UnknownType.prosemirror_to_markdownnaming the policycreate_marked_text(codevs unknown marks)test_unknown_marks_are_skipped_by_design: bare unknown mark, mixed known+unknown, code+link+unknownTests:
cargo test --offline --features web --lib web::markdown— 224 passed, 0 failed (stubweb/dist, gitignored, required for rust-embed compile).Closes #1045.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by CodeRabbit