fix(rss): isolate per-conversion markdown options

[illusion (xjsns346)]

RssConverter stored per-call Markdown options on the converter instance. When the same MarkItDown instance converted two feeds concurrently, one call could overwrite options such as keep_data_uris while the other call was still rendering, so both outputs could use whichever options were written last.

This passes Markdown options explicitly through the RSS/Atom rendering path and adds a synchronized regression test that runs two conversions with opposite keep_data_uris values against one converter instance.

Rebase note: the branch was rebased onto current main, which had moved the RSS tests into test_rss.py; the concurrency regression test is added there (test_concurrent_conversions_keep_call_specific_markdown_options).

Validation:

• python -m pytest packages/markitdown/tests/test_rss.py -q -> 165 passed
• The new test fails on the pre-fix code (keep_data_uris=True result is clobbered by the concurrent False call) and passes after the fix.
• Black 23.7.0 check on the changed files
• git diff --check

[illusion (xjsns346)]

@microsoft-github-policy-service agree

[XU (kokokoXUY)]

Review of #2564 - fix(rss): isolate per-conversion markdown options

Summary

RssConverter used to stash the caller's markdownify options on the converter
instance (self._kwargs = kwargs in convert()), so two conversions running at
the same time on one instance would both render with whichever option set was
written last. This patch deletes that attribute and threads the options through
the call chain as a markdown_options parameter instead, ending in
_CustomMarkdownify(**(markdown_options or {})).

I reproduced the failure deterministically on the pre-patch source, confirmed the
patch fixes it for both feed types and for a second option (heading_style), and
checked that the threading is complete (no self._kwargs left anywhere in the
package). I would merge it.

What the patch does

• packages/markitdown/src/markitdown/converters/_rss_converter.py (+34/-10):
  removes self._kwargs = {} (base line 89) and self._kwargs = kwargs
  (base line 150). convert() now calls
  self._parse_rss_type(doc, strict=strict, markdown_options=kwargs) and
  self._parse_atom_type(doc, strict=strict, markdown_options=kwargs), and
  _parse_atom_type, _render_atom_content, _parse_rss_type and
  _parse_content all take a keyword-only
  markdown_options: dict[str, Any] | None = None. The markdownify instance is
  now built per render call from the argument instead of from instance state.
• packages/markitdown/tests/test_rss.py (+35): a regression test whose converter
  subclass synchronises two threads with threading.Barrier(2) inside
  _parse_content, so the two ThreadPoolExecutor conversions on one instance are
  guaranteed to overlap, then asserts that the keep_data_uris=True conversion
  keeps the embedded data URI and the keep_data_uris=False conversion does not.

Single-threaded behaviour is unchanged: the dict that reaches markdownify is the
same kwargs that convert() received after strict is popped, exactly as before.

What I verified locally (Windows 10/11 checkout, CPython 3.13.12, markdownify 1.2.3)

1. The race is real and I can make it deterministic. On the pre-patch source I
   used the same barrier trick in a separate harness (two threads, one converter
   instance, keep_data_uris True/False and heading_style atx/underlined). The
   keep_data_uris=True call came back with the other call's options -
   {'embedded': False, 'truncated': True, 'atx': False, 'setext': True} - and the
   two results were byte-identical (A and B identical output: True). That is the
   worst form of this bug: one caller silently gets another caller's rendering
   options. The same result holds on the Atom and the RSS rendering paths. Note the
   options are written in convert() before _parse_content runs, so any overlap
   between two calls on one instance loses one caller's options; the barrier only
   removes the timing luck. Without the barrier (32 tasks, 8 workers, alternating
   option sets) I could not trigger it - the window is narrow, which is presumably
   why it went unnoticed.

2. The patch fixes it in both paths. The same harness on the patched source:
   both interleaved conversions return their own options (A and B identical output: False), the stress run reports wrong=0, and a spy on
   _CustomMarkdownify shows exactly one markdownify instance per conversion
   carrying exactly that caller's options. Sequentially, two conversions on one
   RssConverter instance and two MarkItDown().convert_stream(..., file_extension=".atom", keep_data_uris=...)
   calls on one MarkItDown instance both behave correctly.

3. Reachable through the public API, not only by direct converter use.
   MarkItDown builds self._converters once (_markitdown.py:178) and calls
   converter.convert(file_stream, stream_info, **_kwargs) (_markitdown.py:670),
   so every conversion on one MarkItDown instance shares the same RssConverter
   object. Two threads calling convert_stream on one instance is enough.

4. The threading is complete. Grep for markdown_options in the patched file
   gives exactly the chain convert ->
   (_parse_rss_type | _parse_atom_type) -> (_parse_content |
   _render_atom_content -> _parse_content) -> _CustomMarkdownify, with no
   _kwargs reference left (base had three: lines 89, 150, 346). The other paths
   do not need it: _get_flattened_text renders titles with BeautifulSoup
   get_text(), and _get_field_base_url / _resolve_content_links use only local
   variables (checked lines 387-437). A package-wide grep for the
   self.<name> = kwargs pattern shows _rss_converter was the only converter
   doing this; the other self._* assignments are constructor-time configuration.

5. Tests. pytest packages/markitdown/tests/test_rss.py -q on the patched tree:
   165 passed in 1.91s, matching the PR description. Red check: with the patched
   test file copied onto the pre-patch source, the new test fails in 3 out of 3
   runs (deterministic), with
   AssertionError: assert '![Pixel](data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///w==)' in '# Feed\n\n## Entry\n![Pixel](data:image/gif;base64...)'
   at test_rss.py:115; the whole file is 1 failed, 164 passed. I restored the
   worktree afterwards (git status --short clean).

6. Merge state, formatting, CI. Main has moved 5 commits since this PR's
   recorded base (base.sha d9cae89 is an ancestor of the current tip
   4cc9fa1), which matches mergeable_state=behind; none of those commits touch
   _rss_converter.py, _markdownify.py or test_rss.py, git merge-tree reports
   no conflicts, and a synthetic merge of current main with this head runs
   test_rss.py at 165 passed. black 23.7.0 --check is clean for both files. The
   CLA check is green; the tests and pre-commit workflow runs are still
   action_required (first-time contributor), so CI has not actually executed yet.

Notes (none of these block the merge)

A. The regression test only pins one option dimension. It asserts
keep_data_uris; it would still pass if, say, only keep_data_uris were threaded
while another option kept leaking. Adding a second dimension (for example
bullets="+" next to the default "*", or heading_style) makes the test falsify
the general claim in the title, "isolate per-conversion markdown options".
Related, for whoever extends this test: in markdownify 1.2.3 the valid
heading_style values are markdownify.ATX ("atx") and markdownify.SETEXT
("underlined"); "setext" is silently ignored, so a test written with that
string would compare identical outputs and pass for the wrong reason.

B. The barrier test fails loudly if the render path changes. _parse_content
is only reached for markup content (_render_atom_content returns plain text
verbatim). If a future change makes the fixture's content skip that method, the
first thread will sit in barrier.wait(timeout=5) and the test will report a
BrokenBarrierError from a .result() call rather than a useful assertion
failure. A driver that hands both option sets to a wrapper and then compares the
two futures would be more robust to that, and would also remove the 5-second stall.

C. _parse_content is private, but its shape is now part of the call protocol.
Callers inside the module pass markdown_options=; an out-of-tree subclass that
overrides _parse_content(self, content, *, base_url="", strict=False) without
**kwargs will now raise TypeError for any feed with markup. That is acceptable
for a private method, but a sentence in the PR body (or the changelog entry) would
save someone a debugging session.

D. Minor, pre-existing, unchanged by this patch: url stays in kwargs and is
therefore forwarded to markdownify as an unknown option (silently accepted in
1.2.3) even though the converter only reads it for doc.documentURI. Popping it
alongside strict would make markdown_options exactly the set of options
markdownify consumes.

How this was produced

I am an AI assistant; this review was produced by inspecting the PR diff, writing a
standalone harness that forces the interleave the test relies on (barrier subclass,
Atom and RSS fixtures, keep_data_uris x heading_style), running it against the
pre-patch and patched trees, running the RSS test file in both directions
(patched source, then patched tests on pre-patch source), grepping the package for
the instance-state pattern, and rehearsing the merge against current main. I did
not modify the PR branch; the red/green runs happened in separate worktrees. The PR
body discloses that the patch itself was written with Codex; this review is
independent of that.

Verdict

Correct, small, and it removes a real shared-state bug on a documented code path. I
would merge it. The notes above are follow-ups, not blockers.

[illusion (xjsns346)]

Thanks for the thorough review. Applied note A: the concurrency test now varies heading_style (atx vs underlined) and bullets ("+" vs "*") alongside keep_data_uris, and asserts each conversion keeps all of its own options. Used the valid "atx"/"underlined" values you flagged rather than "setext".

Verified: the test fails on the pre-patch RssConverter (all three dimensions leak to the other call), passes on the patched one, test_rss.py is 165 passed, and black --check is clean. Notes B–D are left as follow-ups; note D (url forwarded to markdownify) is pre-existing and didn't seem worth widening this PR.

[illusion (xjsns346)]

Rebased this PR onto the current main branch with no conflicts. The focused RSS test suite passes locally (165 passed), including the deterministic concurrent-conversion regression coverage. When convenient, could a maintainer approve the first-time-contributor Actions run and review the updated branch?