Skip to content

id(web): unknown marks are silently dropped on save while unknown nodes error — pick and pin a policy #1045

New issue
New issue
Closed
#4685
Closed
id(web): unknown marks are silently dropped on save while unknown nodes error — pick and pin a policy#1045
#4685
Assignees
dezren39
Labels
app/idPackage/app: id (pkgs/ or apps/ in this repo)effort/sSmall: expected under 2 hours of workkind/debtTechnical debt worth paying down deliberatelymode/onlineFully doable remotely: design, config, code, cloudpriority/lowNice to have; do when convenientqueue/queuedHas a queued task in the Supabase agent queuesource/creatorFiled by the new-issue-creator jobtask/triageQueue packet kind: verify, reproduce, and route this item

Description

@dezren39
dezren39
opened on Oct 27, 2026

Papercut / inconsistency

The save direction of the id web markdown codec treats unknown names inconsistently:

  • Unknown node type → hard error: ConversionError::UnknownType (pkgs/id/src/web/markdown.rs:1186). The caller learns the document could not be converted faithfully.
  • Unknown mark type → silently skipped: create_marked_text (markdown.rs:1442-1459) matches em / strong / strikethrough / link and falls through with _ => continue, // Skip code (handled below) and unknown marks. The text survives; the formatting vanishes with no error and no signal.

Evidence (verified 2026-10-26 on main cf60812)

Standalone probe crate carrying a verbatim copy of src/web/markdown.rs (comrak 0.56.0 locked) — log in goal hidden_files probe-2026-10-26-2145/out3.log:

  • SAVE unknown_node (doc containing a mention node) → ERR=UnknownType("mention")
  • SAVE unknown_mark (paragraph text hi carrying an underline mark) → ok="hi\n" — mark gone, no error

Why it matters

ProseMirror documents reach this codec from the editor, collab peers, imports, and API writers — producers whose mark vocabularies can be larger than the codec's four-plus-code (underline and highlight are the obvious examples). Today a document using one saves "successfully" while quietly losing that formatting everywhere it appears. Either behavior can be defended — erroring like nodes, or skipping marks by design — but the split is currently accidental, undocumented at the API level, and untested: the continue arm's comment lumps deliberate code handling together with arbitrary unknown marks.

Acceptance criteria

  • A documented, tested policy for unknown marks: either Err(ConversionError::…) (consistent with unknown nodes), or an explicitly documented skip with a regression test pinning the behavior and a doc comment on prosemirror_to_markdown naming it.
  • The code mark's special handling stays intact and covered by existing tests.
  • Existing web::markdown tests still pass; documents using only known marks render byte-identically to before.

Activity

  1. developing-today commented on Oct 27, 2026

    @developing-today
    developing-todayBot
    on Oct 27, 2026 – with developing.today
    Contributor

    CODE-190

  2. github-actions
    assigned
    dezren39
    on Oct 27, 2026
  3. dezren39
    added
    kind/debtTechnical debt worth paying down deliberately
    task/triageQueue packet kind: verify, reproduce, and route this item
    status/needs-triageNot yet triaged; needs a first look
    source/creatorFiled by the new-issue-creator job
    priority/lowNice to have; do when convenient
    queue/queuedHas a queued task in the Supabase agent queue
    app/idPackage/app: id (pkgs/ or apps/ in this repo)
    on Oct 27, 2026

  4. dezren39 commented on Oct 27, 2026

    @dezren39
    dezren39
    on Oct 27, 2026
    MemberAuthor

    🤖 Agent note · filed

    • Agent: new-issue-creator job
    • Session / ID: new-issue-creator cron run 2026-10-26 21:45 CDT
    • Found via: filed this run
    • Task: Verified with a standalone probe crate carrying a verbatim copy of src/web/markdown.rs (comrak 0.56.0) on main cf60812 (log hidden_files/probe-2026-10-26-2145/out3.log): an unknown node type returns Err(UnknownType) (markdown.rs:1186) while an unknown mark (underline) is silently dropped by create_marked_text's fallthrough arm (markdown.rs:1459, '_ => continue'), returning Ok with the formatting gone. Enqueued as a triage packet to repo-health-code-pilot — the ask is a decided, documented, tested policy, not a prescribed direction.
    • Expectation: triage picks error-vs-documented-skip; done when the policy is implemented/documented with a pinning regression test.
    • Links: probe evidence in goal hidden_files (see issue body)
  5. dezren39
    added
    effort/sSmall: expected under 2 hours of work
    mode/onlineFully doable remotely: design, config, code, cloud
    queue/claimedQueue worker lifecycle state
    and removed
    status/needs-triageNot yet triaged; needs a first look
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 1, 2026

  6. 14 remaining items

  7. dezren39 commented on Nov 3, 2026

    @dezren39
    dezren39
    on Nov 3, 2026
    MemberAuthor

    🤖 Agent note · handed off

  8. dezren39 commented on Nov 3, 2026

    @dezren39
    dezren39
    on Nov 3, 2026
    MemberAuthor

    agent-note:v1 — state: handed off

    Queue task #6632 is handed to OpenCode for execution.

    • Session: ses_e5e6134b8c6ffeqSR2YJ9QOaH7
    • Lane: opencode
    • Worker claim: repo-worker-code
    • Task: pin the documented skip policy for unknown marks; scope and acceptance are in task #6632.
  9. dezren39
    added
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    status/parkedDeliberately paused with a recorded resume condition
    and removed
    status/in-progressQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    work/assignedQueue worker lifecycle state
    on Nov 3, 2026

  10. dezren39 commented on Nov 3, 2026

    @dezren39
    dezren39
    on Nov 3, 2026
    MemberAuthor

    🤖 Agent note · superseded

  11. dezren39
    added
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 3, 2026

  12. dezren39 commented on Nov 3, 2026

    @dezren39
    dezren39
    on Nov 3, 2026
    MemberAuthor

    agent-note:v1
    state: triaged (coverage pass)
    Coverage pass 2026-11-03: reviewed in the fleet-wide per-issue pass (drift / overlap / already-handled / stale / blockers / ordering). Verdict: still live; queued in repo-health-code-pilot as triage at priority 30 (priority/low). If this is already handled at current main, the triage closes it with evidence; if it needs splitting, the triage files the split.
    Found via: coverage pass 2026-11-03

  13. dezren39
    added a commit that references this issue on Nov 3, 2026

    Merge pull request #4685 from developing-today/queue/6632-1045-unknow…

    9a1fe42
  14. dezren39
    closed this as completedin #4685on Nov 3, 2026

  15. dezren39 commented on Nov 3, 2026

    @dezren39
    dezren39
    on Nov 3, 2026
    MemberAuthor

    🤖 Agent note · done — policy pinned and merged

    • Agent: repo-worker-code (OpenChamber scheduled lane)
    • Session / ID: queue task 7606, repo-health-code-pilot, lane 9aca43b4-5439-4939-9f71-323f5d32521b
    • Found via: atomic queue claim (2026-11-03 coverage pass; packet noted the queue/parked label with no live task)
    • Decision: documented skip-by-design, not an error. Rationale: an unknown inline mark can be shed without corrupting text (the node case corrupts structure), and erroring would break every producer whose mark vocabulary is larger than the codec's — a breaking change with no data-loss benefit. Recorded in the doc comment on prosemirror_to_markdown, the clarified _ => continue arm in create_marked_text, and a regression test pinning bare / mixed / code+link+unknown combinations.
    • Disposition: PR fix(id): pin documented skip policy for unknown marks (#1045) #4685 (queue/6632-1045-unknown-mark-policy, head 1b5d79e1, 43+/1- in pkgs/id/src/web/markdown.rs) was in flight but idle ~9h with no live owner; verified rather than duplicated, then merged as 9a1fe429c8b4b6ece5fcdf1587dfdf022c63ffe6.
    • Verification: cargo test -p id --features web --lib unknown_marks → 1 passed; cargo test -p id --features web --lib web::markdown → 224 passed, 0 failed (worktree ~/git/fleet/7606-1045). Note: pkgs/id has no PR-tier CI workflow trigger on main today (.github/workflows/id-check.yml runs on push/schedule only), so the local run is the verification of record for this merge.
    • Labels: removed stale queue/parked + status/parked; no hard-stop labels were present.
    • Follow-up: none required for the codec. The PR-tier CI gap is a separate repo-health observation, not filed here.
  16. dezren39
    removed
    status/parkedDeliberately paused with a recorded resume condition
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    on Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/idPackage/app: id (pkgs/ or apps/ in this repo)effort/sSmall: expected under 2 hours of workkind/debtTechnical debt worth paying down deliberatelymode/onlineFully doable remotely: design, config, code, cloudpriority/lowNice to have; do when convenientqueue/queuedHas a queued task in the Supabase agent queuesource/creatorFiled by the new-issue-creator jobtask/triageQueue packet kind: verify, reproduce, and route this item

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions