fix(channels/telegram): keep reply-threads in the main chat conversation history

[metalmon]

Maintainer's note (Audacity88): I rewrote this description on behalf of @metalmon to match the current template and exact-head public evidence, and I linked Closes #10237 because this patch implements that issue's complete history-key correction. The original commit a381970a9d3cfc137d8a01a2cee86c634a42c3e7 and description remain in the branch history; authorship of the code remains with @metalmon. This edit does not change the implementation or scope.

Summary

• Base branch: master
• What changed and why: Telegram supplies message_thread_id for genuine forum topics and ordinary reply gestures. The inbound parsers previously treated both as topic boundaries, so ordinary replies forked conversation history away from the main chat.
• What changed and why: A shared topic_thread_id helper now derives topic identity only when is_topic_message: true, preserving real topic isolation while ordinary replies retain the main-chat history.
• Scope boundary: This does not change sender/chat isolation, permissions, approval or tool gates, Bot API networking, or introduce Telegram reply_parameters presentation behavior.
• Blast radius: Telegram text, attachment, and voice inbound parsing plus the conversation-history keys derived from their reply_target and thread_ts fields.
• Linked issue(s): Closes [Bug]: Telegram reply-threads fragment conversation memory into per-thread history buckets #10237.
• Labels: bug, channel, experienced contributor, channel:telegram, risk:high, size:S, needs-maintainer-review

Testing (required)

How you can test (when useful)

• Reviewer testing requested? N/A; the parser-level regression exercises the exact Telegram update fields and exact-head CI covers all three changed parser call sites.

How I tested

• CI checks relied on and why they cover this change: Exact-head required CI passed, including Test, Parallel Runtime Test, Lint, the workspace feature checks, platform builds/checks, and CI Required Gate.
• Known CI coverage gap, if any: No live Telegram chat smoke was run. Automated coverage reaches the production update parser and history-identity fields; actual Bot API reply rendering remains outside this history-scope change.
• Commands run and tail output:

cargo nextest run --locked -p zeroclaw-channels --features channel-telegram --lib
# 1494 tests run; 1494 passed; 2 skipped

ZEROCLAW_PARALLEL_TEST_SCOPE=channels ZEROCLAW_PARALLEL_TEST_RUNS=3 ZEROCLAW_PARALLEL_TEST_THREADS=16 ./scripts/ci/parallel_runtime_test_gate.sh
# all 3 runs: 1494 passed; 0 failed; 2 ignored

cargo clippy --locked --workspace --exclude zeroclaw-desktop --all-targets --features ci-all -- -D warnings
# passed with warnings denied

cargo check --locked --workspace --exclude zeroclaw-desktop --features ci-all
# passed

cargo fmt --all -- --check
# passed

• Beyond CI, what did you manually verify? Reviewed the exact-head Telegram update parsing and confirmed that an ordinary reply has the same reply_target and thread_ts as a plain message, while an update marked is_topic_message: true retains its topic ID. No live Telegram chat smoke was performed.
• Visual interface changed? N/A; this changes conversation-history identity, not rendered layout or presentation.
• If any command was intentionally skipped, why: No additional live Bot API test was run because the changed behavior is the deterministic parsing of Telegram update fields into conversation-history identity.

Security & Privacy Impact (required)

• New permissions, capabilities, or file system access scope? No
• New external network calls? No
• Secrets / tokens / credentials handling changed? No
• PII, real identities, or personal data in diff, tests, fixtures, or docs? No
• Prompt injection or untrusted model-visible text introduced/changed? No
• If any Yes, describe the risk and mitigation: N/A

Compatibility (required)

• Backward compatible? Yes
• Config / env / CLI surface changed? No
• Rust/MSRV/toolchain floor changed? No
• If backward compatibility is No or either surface/floor question is Yes: N/A

Rollback (required for medium/high-risk PRs)

• Fast rollback command/path: After merge, revert the landed squash commit for PR fix(channels/telegram): keep reply-threads in the main chat conversation history #10418.
• Feature flags or config toggles: None.
• Observable failure symptoms: Ordinary Telegram replies again lose prior conversation context, or genuine forum-topic messages unexpectedly share main-chat history.

[zeroclaw-reviewer]

Review context

Head reviewed: a381970

I reviewed the full origin/master...HEAD diff, the surrounding Telegram parser and outbound call paths, the conversation_history_key caller, the current PR body, the current issue and label metadata, and the existing review/conversation state. PR #10418 is open and mergeable, but it is currently blocked on the required review. There are no prior formal reviews, inline comments, or top-level review comments, so there are no settled concerns to re-raise.

The central approach is sound: Telegram's is_topic_message is the correct discriminator for a forum topic, and centralizing the decision in topic_thread_id avoids three copies of subtly different parsing logic. The remaining issues are coverage and review-contract gaps, plus one behavior trade-off worth making explicit.

🔴 Blocking — Cover the attachment and voice parser boundaries

The new helper at crates/zeroclaw-channels/src/telegram.rs:2729-2741 is wired into three independently constructed ChannelMessage paths: the attachment parser at :2205, the voice parser at :2435, and the text parser at :2785. All three feed conversation_history_key, which treats a non-None thread_ts as a history boundary (crates/zeroclaw-channels/src/orchestrator/mod.rs:638-658).

The added regression at telegram.rs:5835-5886 exercises only parse_update_message. The successful attachment tests at :10541, :10607, and :10674 do not provide message_thread_id or is_topic_message, and the voice tests at :7495, :7517, and :7547 stop before the successful parsed-message return (disabled transcription, over-limit input, or unauthorized sender). A future drift in either media parser could therefore reintroduce the exact history fragmentation this PR fixes while the new text test remains green.

Please add successful-path regressions for both attachment and voice parsing that cover (a) message_thread_id with no or false is_topic_message producing the main-chat reply_target and thread_ts: None, and (b) a genuine topic with is_topic_message: true retaining chat_id:thread_id and thread_ts. Where practical, assert the resulting conversation_history_key directly, not only the intermediate fields. This is a behavioral boundary, not a style preference: it is the only protection against the three parser paths drifting apart again.

🔴 Blocking — Complete the required PR description and validation contract

The live body currently contains only ## Summary, ## Root cause, ## Fix, ## Testing, and ## Security / privacy. It does not satisfy the current .github/pull_request_template.md contract: the required ## Testing (required), ## Security & Privacy Impact (required), and ## Compatibility (required) sections are absent, and the medium/high-risk rollback section is absent if this channel-history behavior is classified at that level. The body also supplies no exact commands, CI checks relied on, relevant output tails, known coverage gap, or manual-verification statement.

This is substantive rather than cosmetic because the missing How I tested evidence makes it impossible to tell from the public artifact whether the attachment and voice paths were considered, and the missing compatibility/security answers leave reviewer confidence dependent on inference. Please fill the current template before approval, including the honest note about the focused media-coverage gap above and the local/CI evidence recorded below. Issue #10237 is an exact match for the reported and fixed behavior and remains open; use Closes #10237 if this PR is intended to resolve it, or explain the non-closing relationship otherwise. The live PR labels are currently channel and channel:telegram; include the required live label snapshot and have the maintainer add any applicable risk/size labels rather than leaving that state implicit.

🔵 Suggestion — Separate history identity from Telegram reply presentation

ChannelMessage::thread_ts is documented as the platform thread identifier used to post threaded responses (crates/zeroclaw-api/src/channel.rs:344-346). After this change, an ordinary Telegram reply has thread_ts: None and a bare chat reply_target. The Telegram send paths derive message_thread_id only from a chat_id:thread_id recipient or an explicit SendMessage.thread_ts (crates/zeroclaw-channels/src/telegram.rs:3009-3089, :4302-4347), and they do not map SendMessage::in_reply_to to Telegram reply_parameters. Thus the history fix also changes an ordinary reply's presentation: the generated response will no longer carry the inbound reply/thread target through these paths.

If that top-level delivery is intentional, document and test that this PR is history-only. If Telegram reply presentation should remain intact, carry the delivery anchor separately from the history-scope identity and use Telegram's reply parameters for ordinary replies, while retaining thread_ts only for actual forum topics. This is not a request to undo the history fix; it is a request to make the user-visible trade-off deliberate.

🟢 What looks good — The discriminator is centralized and preserves forum isolation

Using one topic_thread_id helper for text, attachments, and voice keeps the platform rule in one place and retains topic isolation when Telegram explicitly marks is_topic_message: true. Updating the existing forum fixture to include that field and asserting thread_ts remains populated is a useful guard against accidentally collapsing real forum topics while fixing ordinary replies. The patch does not alter allowlist, approval, tool-gating, or network behavior.

Validation

• git diff --check origin/master...HEAD — passed.
• cargo fmt --all -- --check — passed.
• cargo build --profile ci --locked --features channel-telegram — passed.
• cargo clippy --locked -p zeroclaw-channels --all-targets --features channel-telegram -- -D warnings — passed.
• CI-equivalent cargo clippy --locked --workspace --exclude zeroclaw-desktop --all-targets --features ci-all -- -D warnings — passed.
• cargo check --locked --workspace --exclude zeroclaw-desktop --features ci-all — passed.
• RUSTFLAGS=-D warnings cargo check --locked --workspace --exclude zeroclaw-desktop --no-default-features — passed.
• RUSTFLAGS=-D warnings cargo check --locked --workspace --exclude zeroclaw-desktop --all-targets — passed.
• cargo nextest run --locked -p zeroclaw-channels --features channel-telegram --lib — passed: 1494 tests run, 1494 passed, 2 skipped.
• The changed-crate parallel runtime gate ran three times with 16 test threads; each run passed 1494 tests, with 0 failures and 2 ignored tests.
• The first full cargo test --locked -p zeroclaw-channels --features channel-telegram --lib run hit tts::tests::edge_tts_reaper_confirms_hard_kill_exit_before_removing_artifact with Linux ExecutableFileBusy in the untouched TTS test. I investigated it: the exact test passed on the PR head when rerun, passed on origin/master, and the complete crate test run passed on rerun with 1494 passed, 0 failed, and 2 ignored. This was an unrelated transient test failure, not a PR regression.
• Fresh GitHub status results for this head were successful for the required aggregate gate, format, lint, build/check, test, parallel-runtime, security, MSRV, and feature-check coverage. Optional platform/docs/Nix checks reported skipped where their path filters did not select them.
• No live Telegram chat smoke test was performed. The validation above covers the parser and workspace boundaries; live Bot API delivery and actual reply rendering remain outside this local evidence.

Suggested verdict: request changes, primarily for the missing media success-path regressions and the incomplete required PR/validation body.

[Audacity88]

@metalmon

I reviewed exact head a381970a9d3cfc137d8a01a2cee86c634a42c3e7, the complete one-file diff, all three Telegram parser paths, the shared history-key consumer, #10237, the exact-head bot review, and current checks.

This is ready. I narrowed the bot feedback after applying the sole-remaining-concern test. The PR body gaps are deterministic maintainer cleanup, so I am correcting them rather than handing them back to you. The requested attachment and voice test matrix would add future-drift hardening, but it is not required to prove this patch: all three paths use the same topic_thread_id predicate, and the text regressions exercise both ordinary replies and genuine topics through a production parser.

The reply-presentation suggestion is also separate from this fix. Ordinary replies are no longer misrepresented as forum topics; adding Telegram reply_parameters would be a distinct presentation feature rather than a condition of restoring the correct history scope.

🟢 What looks good — One discriminator now owns Telegram topic identity

The helper makes is_topic_message: true the only route to topic-scoped reply_target and thread_ts, so ordinary reply gestures keep the main-chat conversation while genuine forum topics remain isolated. The changed attachment, voice, and text paths all use that same rule, and the exact-head focused, parallel, workspace, feature, and required CI evidence is green.

This is approved, but per FND-003 it still requires a second independent Core Team approval before merge.

[zeroclaw-reviewer]

Review context

Head reviewed: cf117a3

I reviewed the complete one-file origin/master...HEAD change, the three Telegram parser paths and their shared downstream consumers, the current PR body and #10237, all conversation state, the current check-run state, and the validation results for this head.

The existing top-level review identified missing attachment and voice success-path coverage on a381970. That blocker is addressed by the new head: both media parsers now exercise genuine topics and ordinary reply threads through their successful return paths. The only formal review is @Audacity88's approval of the previous head; there are no inline review threads. The current PR body contains every applicable template section, its label snapshot matches the live labels, and Closes #10237 accurately describes the complete issue fix.

The current head's GitHub checks are successful, including Test, Parallel Runtime Test, Lint, the feature/default/no-default check matrix, the Linux build, platform checks, security, and CI Required Gate. The PR remains a risk:high change, so the required human-approval policy still applies independently of this draft.

🔴 Blocking — Remove prohibited AI attribution from the new commit

Commit cf117a3 contains an AI co-author trailer and a generated session footer. That violates the repository's commit-message hygiene requirement in AGENTS.md:33 and the explicit prohibition in .github/pull_request_template.md:95-98. It also puts generated-tool provenance into a permanent public Git artifact rather than the review trail.

@metalmon, please amend or replace this unpublished commit with the same code and tests but without AI co-author or generated-session trailers. Preserve only any permitted human attribution. This is a merge blocker because the project contract forbids those footers.

🟡 Warning — Refresh the public test-count evidence for the new head

The PR body's How I tested output reports 1494 passing channel tests, but cf117a3 adds two Telegram parser tests after the reviewed a381970 head. The same focused command now reports 1496 passed and 2 ignored, and exact-head CI is green. Update the body’s command tail to reflect the current result so the public record follows the evidence requirements in .github/pull_request_template.md:29-54.

This is non-blocking because the fresh current-head CI and local validation cover the added tests; the requested change is to make the public validation trail accurate.

🟡 Warning — Use a decodable JPEG fixture for the attachment regression

crates/zeroclaw-channels/src/telegram.rs:5900 serves six bytes that only resemble a JPEG header. The parser test currently needs only the downloaded attachment path, so it passes, but the fixture is not a decodable JPEG. Replace it with a valid minimal JPEG or a shared generated image fixture.

This is the same regression class recorded in institutional finding 2026-08-27-pr-9819-discord-invalid-jpeg-fixture-regression: header-only image fixtures become misleading or fail for unrelated reasons when image validation tightens. It is non-blocking here because the production behavior and the topic-thread boundary are independently covered, but fixing the fixture keeps the test focused on the behavior it names.

✅ Resolved — Media parser coverage now reaches both history boundaries

The earlier coverage concern is resolved on this head. TelegramChannel::topic_thread_id is used by the attachment, voice, and text parsers at crates/zeroclaw-channels/src/telegram.rs:2205, :2435, and :2785. The new successful-path regressions at :5888-5958 and :5961-6043 verify that is_topic_message: true preserves chat_id:thread_id plus thread_ts, while ordinary reply threads remain on the bare chat target with thread_ts: None.

That matches the actual history-key boundary in crates/zeroclaw-channels/src/orchestrator/mod.rs:638-657: with the same sender, channel alias, reply target, and no thread timestamp, ordinary replies and plain messages share one sender-scoped history key. The focused attachment and voice tests passed under both the standard harness and nextest.

✅ Resolved — Reply presentation is explicitly out of scope for this fix

The current PR body states that Telegram reply_parameters presentation is not part of this change. I verified that the outbound Telegram path still derives forum routing from the recipient/thread value at crates/zeroclaw-channels/src/telegram.rs:4302-4346; this patch intentionally changes only which inbound messages receive a forum-topic thread_ts. That makes the prior presentation question a separately scoped enhancement rather than an implicit, unreviewed behavior change in this bug fix.

🟢 What looks good — The discriminator is centralized at the right boundary

topic_thread_id at crates/zeroclaw-channels/src/telegram.rs:2723-2741 makes is_topic_message: true the only condition that creates topic-scoped routing and history identity. This preserves intended forum isolation while preventing ordinary reply gestures from splitting context. Keeping the decision shared across text, attachment, and voice parsers avoids the three paths drifting into subtly different session behavior.

Validation

• git diff --check origin/master...HEAD — passed.
• cargo fmt --all -- --check — passed.
• cargo build --profile ci --locked --features channel-telegram — passed.
• cargo clippy --locked -p zeroclaw-channels --all-targets --features channel-telegram -- -D warnings — passed.
• cargo test --locked -p zeroclaw-channels --features channel-telegram --lib topic_thread_gate — passed: 2 passed.
• cargo nextest run --locked -p zeroclaw-channels --features channel-telegram --lib topic_thread_gate --no-fail-fast — passed: 2 passed.
• cargo test --locked -p zeroclaw-channels --features channel-telegram --lib — passed: 1496 passed, 2 ignored.
• ZEROCLAW_PARALLEL_TEST_SCOPE=channels ZEROCLAW_PARALLEL_TEST_RUNS=3 ZEROCLAW_PARALLEL_TEST_THREADS=16 ./scripts/ci/parallel_runtime_test_gate.sh — passed all three runs: each 1496 passed, 2 ignored.
• cargo clippy --locked --workspace --exclude zeroclaw-desktop --all-targets --features ci-all -- -D warnings — passed.
• cargo check --locked --workspace --exclude zeroclaw-desktop --features ci-all — passed.
• RUSTFLAGS=-D warnings cargo check --locked --workspace --exclude zeroclaw-desktop --no-default-features — passed.
• RUSTFLAGS=-D warnings cargo check --locked --workspace --exclude zeroclaw-desktop --all-targets — passed.
• A local full workspace nextest run stopped at unchanged skills::install_location_tests::install_error_strings_resolve_through_fluent in src/skills/mod.rs:1317; the same focused test fails on origin/master, so it is pre-existing and unrelated to this PR.
• A first full channels nextest run reported three unchanged, time-sensitive Matrix typing tests in crates/zeroclaw-channels/src/orchestrator/mod.rs:16657 and :23025. All three passed when rerun individually, and the full channels nextest suite passed on origin/master; this is not evidence of a Telegram regression.
• Fresh GitHub check runs for cf117a3 are successful, including CI Required Gate, Test, Parallel Runtime Test, Lint, feature/default/no-default checks, Linux build, platform checks, MSRV, security, and the applicable matrix jobs.

Suggested verdict: request changes for the prohibited commit-message attribution. The two warnings can be corrected with that cleanup but do not independently block the Telegram behavior fix.

[zeroclaw-reviewer]

Review context

Head reviewed: 0dce9ec

I reviewed the complete one-file origin/master...HEAD diff, the surrounding Telegram attachment, voice, and text parser paths, the shared conversation_history_key consumer, the current PR body and labels, issue #10237, all top-level conversation state, inline threads, formal reviews, and the current check state.

PR #10418 is open and mergeable. The existing formal approval is from @Audacity88 on the previous a381970a9d3cfc137d8a01a2cee86c634a42c3e7 head. There are no inline review threads. The earlier media-coverage and required-description gaps were addressed by the current revision. The intermediate bot comment about commit attribution was for a different cf117a380b43a49f9581b861e0243468bbbd6b42 revision that is not in the current two-commit ancestry; the current commits contain no prohibited attribution trailers. The linked issue is an exact match for this fix, and Closes #10237 is accurate.

The central implementation remains sound: topic_thread_id makes is_topic_message: true the discriminator for forum-topic identity, and the same helper is used by the text, attachment, and voice parsers. The current added tests reach the successful return paths for both media parsers and exercise both the real-topic and ordinary-reply boundaries.

🟡 Warning — Refresh the stale test-count evidence

The live PR body's How I tested section still reports 1494 passing channel tests for both the focused cargo nextest command and the three-run parallel gate. The current head adds two tests in telegram.rs, and the exact current commands report 1496 passed and 2 ignored. Please update both public output tails to match the evidence from 0dce9ec.

This is non-blocking because exact-head GitHub CI is green and the current local focused and parallel runs cover the added tests. It is still worth correcting because .github/pull_request_template.md:29-54 requires the public validation record to describe the checks and results actually relied upon.

🟡 Warning — Use a decodable JPEG fixture for the attachment regression

The new attachment regression at crates/zeroclaw-channels/src/telegram.rs:5900 serves only six bytes, FF D8 FF E0 01 02, as photos/file_1.jpg, then asserts a successful photo parse. The current path succeeds because the saved .jpg path is enough for the marker/classification logic, but those bytes are not a complete decodable JPEG. If attachment preparation later validates or decodes the saved image, this test will fail for an unrelated fixture error instead of testing the topic-thread boundary.

Please reuse the existing valid fixture at crates/zeroclaw-channels/tests/fixtures/test_photo.jpg or generate a minimal valid JPEG while keeping the test focused on history identity. This is non-blocking test-quality hardening, not a production behavior regression.

✅ Resolved — Media parser coverage now reaches both history boundaries

The earlier review's blocking coverage concern on a381970a9d3cfc137d8a01a2cee86c634a42c3e7 is resolved on this head. topic_thread_id is used by the attachment, voice, and text parsers at telegram.rs:2205, :2435, and :2785. The new successful-path regressions at telegram.rs:5888-5959 and :5961-6044 verify that a genuine forum topic retains chat_id:thread_id plus thread_ts, while an ordinary reply-thread without is_topic_message uses the bare chat target and thread_ts: None.

That matches the downstream history boundary in crates/zeroclaw-channels/src/orchestrator/mod.rs:638-657: with sender-scoped history, the ordinary reply and a plain message now produce the same history identity inputs. The feature-isolated tests also pass, so the new regressions are not only compiled through the default workspace feature set.

✅ Resolved — Required PR template and public metadata are present

The current PR body contains the required Summary, Testing, Security & Privacy Impact, Compatibility, and medium/high-risk Rollback sections. It includes exact commands, CI coverage, known gaps, and the no-live-Bot-API-smoke explanation. Its label snapshot matches the live labels (bug, channel, experienced contributor, channel:telegram, risk:high, size:S, and needs-maintainer-review), and the issue relationship uses the accurate closing verb.

The remaining public-artifact issue is limited to the stale test counts called out above.

✅ Resolved — Reply-presentation scope is explicit

The earlier question about whether collapsing ordinary reply threads would also change Telegram reply presentation is now explicitly scoped in the PR body: this fix changes conversation-history identity and does not introduce reply_parameters behavior. The outbound send path remains unchanged and still derives forum routing from an explicit chat_id:thread_id recipient. If reply presentation is expanded later, it should be a separate, independently tested change rather than an implicit part of this history fix.

🟢 What looks good — One discriminator owns Telegram topic identity

Making is_topic_message: true the only route to a topic-scoped reply_target and thread_ts is the right boundary for the issue being fixed. It preserves intentional forum-topic isolation while preventing ordinary reply gestures from creating a new conversation-history bucket. Keeping this decision in one helper and using it in all three inbound parser paths avoids future drift between text, attachment, and voice handling.

Verdict

Approve. The two 🟡 items are non-blocking public-record and fixture-quality warnings; I found no remaining substantive blocker in the current implementation. This remains a risk:high PR, so the project's independent Core Team approval requirement applies separately from this review.

Validation

• git diff --check origin/master...HEAD — passed.
• cargo fmt --all -- --check — passed.
• cargo build --profile ci --locked --features channel-telegram — passed; Finished ci in 12m27s.
• cargo clippy --locked -p zeroclaw-channels --all-targets --features channel-telegram -- -D warnings — passed; Finished dev in 8m07s.
• cargo test --locked -p zeroclaw-channels --features channel-telegram --lib — passed: 1496 passed, 0 failed, 2 ignored.
• cargo nextest run --locked -p zeroclaw-channels --features channel-telegram --lib — passed: 1496 tests run, 1496 passed, 2 skipped.
• cargo build --profile ci --locked --no-default-features --features channel-telegram — passed; Finished ci in 7m55s.
• cargo clippy --locked -p zeroclaw-channels --no-default-features --all-targets --features channel-telegram -- -D warnings — passed; Finished dev in 9m27s.
• cargo test --locked -p zeroclaw-channels --no-default-features --features channel-telegram --lib — passed: 878 passed, 0 failed, 2 ignored.
• CI-equivalent cargo clippy --locked --workspace --exclude zeroclaw-desktop --all-targets --features ci-all -- -D warnings — passed; Finished dev in 5m23s.
• cargo check --locked --workspace --exclude zeroclaw-desktop --features ci-all — passed; Finished dev in 4m43s.
• RUSTFLAGS="-D warnings" cargo check --locked --workspace --exclude zeroclaw-desktop --no-default-features — passed; Finished dev in 2m45s.
• RUSTFLAGS="-D warnings" cargo check --locked --workspace --exclude zeroclaw-desktop --all-targets — passed; Finished dev in 2m53s.
• ZEROCLAW_PARALLEL_TEST_SCOPE=channels ZEROCLAW_PARALLEL_TEST_RUNS=3 ZEROCLAW_PARALLEL_TEST_THREADS=16 ./scripts/ci/parallel_runtime_test_gate.sh — passed all three runs; each reported 1496 passed, 0 failed, and 2 ignored.
• cargo nextest run --locked --workspace --exclude zeroclaw-desktop — failed at the unchanged zeroclaw skills::install_location_tests::install_error_strings_resolve_through_fluent test in src/skills/mod.rs:1317, with cli-skills-install-git-failed did not resolve/interpolate: échec de l'installation de la source de compétence git : acme/widget. The same exact test was rerun on a detached origin/master worktree and failed with the same output, so this is a pre-existing environment/source failure unrelated to the Telegram-only diff; the focused channels suite passed.
• Fresh GitHub checks for this head report CI Required Gate, Test, Lint, build/check matrices, Parallel Runtime Test, MSRV, Security, and applicable platform checks as passed. Docs Style, Nix, Web Permission, and targeted Windows Clippy checks are skipped by their path filters; no skipped check covers the changed Telegram Rust surface.
• No live Telegram chat smoke test was performed. The deterministic parser and full media success-path tests cover the changed history identity behavior; live Bot API delivery and future reply-presentation behavior remain outside this change's scope.

[zeroclaw-reviewer]

Review context

Head reviewed: 624ba4a

I reviewed the complete origin/master...HEAD diff, the surrounding Telegram attachment, voice, and text parser paths, the downstream conversation_history_key consumer, the current PR body and labels, issue #10237, all available conversation state, formal reviews, inline threads, and the exact-head check status.

PR #10418 is open and mergeable. The only formal approval is @Audacity88's approval on the earlier implementation head; there are no inline review threads or active CHANGES_REQUESTED formal reviews. The earlier coverage concern is addressed by the current test commit: the attachment and voice tests now reach their successful parsed-message returns and exercise both genuine forum topics and ordinary reply-threads. The current body also contains the required template sections, its label snapshot matches the live labels, and Closes #10237 is an accurate relationship. The remaining issues are non-blocking public-evidence and fixture-quality cleanup.

🟡 Warning — Refresh the stale channel test counts in the PR body

The live PR body's How I tested subsection still reports 1494 passing channel tests for both its focused cargo nextest command and its three-run parallel gate. The current head adds the attachment and voice regression tests at crates/zeroclaw-channels/src/telegram.rs:5908-6064; the exact current-head focused suite reports 1496 passed and 2 ignored. Please update both public output tails so the validation record describes the revision actually under review, as required by .github/pull_request_template.md:29-54.

This is non-blocking. The exact-head GitHub required gate is green, and the current local focused test, full channels test, and nextest runs cover the added tests successfully. The correction is needed for an auditable public record, not because the implementation is failing.

🟡 Warning — Make the replacement JPEG fixture actually decodable

The new tiny_jpeg() fixture at crates/zeroclaw-channels/src/telegram.rs:5888-5906 is documented as a real, decodable JPEG. A direct decode through the repository's built image crate rejects these exact bytes with Error decoding huffman values: Excessive Huffman table of length 256 found when header length is 1. The current try_parse_attachment_message path saves and wraps the downloaded bytes without decoding them, so the topic-thread assertions pass while the fixture remains malformed.

Please replace the hand-written bytes with a generated or otherwise verified JPEG, and validate it with image::load_from_memory_with_format before relying on the fixture. This is non-blocking because the production change is the shared topic_thread_id predicate and the parser behavior under test is independent of image decoding; it is still worth fixing so a future image-validation change does not turn this regression into an unrelated fixture failure.

✅ Resolved — Attachment and voice coverage now reaches the changed boundaries

The prior blocking coverage concern on the earlier implementation head is resolved here. The shared helper at crates/zeroclaw-channels/src/telegram.rs:2729-2741 is used by the attachment, voice, and text paths at :2205, :2435, and :2785. The new successful-path tests at :5908-6064 verify that is_topic_message: true preserves chat_id:thread_id and thread_ts, while a message_thread_id without that flag stays on the bare chat target with thread_ts: None.

Those assertions match the downstream sender-scoped history boundary in crates/zeroclaw-channels/src/orchestrator/mod.rs:638-657: ordinary replies and plain messages now have the same history-key inputs, while genuine forum topics remain isolated.

✅ Resolved — Required PR template and public metadata are present

The current body includes Summary, Testing, Security & Privacy Impact, Compatibility, and the required medium/high-risk Rollback sections. It names the checks relied on, the known no-live-Bot-API gap, and the exact scope boundary. The live label set matches the body's snapshot (bug, channel, experienced contributor, channel:telegram, risk:high, size:S, and needs-maintainer-review), and the linked issue is an exact match for the fix. The stale test counts above are the remaining public-artifact correction.

✅ Resolved — Reply-presentation scope is explicit

The earlier question about Telegram reply presentation is now clearly scoped in the PR body: this change corrects conversation-history identity and does not introduce reply_parameters behavior. The outbound path remains unchanged, so any future enhancement to preserve ordinary reply presentation can be reviewed independently rather than being silently bundled into this history fix.

🟢 What looks good — One discriminator owns Telegram topic identity

Making is_topic_message: true the only route to topic-scoped reply_target and thread_ts is the right boundary for #10237. It preserves deliberate forum-topic isolation while preventing ordinary reply gestures from creating a separate conversation-history bucket. Centralizing that rule and using it in all three inbound parser paths avoids future drift between text, attachment, and voice handling. The patch does not alter allowlists, approval gates, tool gates, or production network behavior.

Verdict

There is no blocking implementation defect in the current head. The two 🟡 findings are non-blocking warnings about stale public counts and a malformed test fixture. Suggested review action: --comment; refresh the public counts and fixture when convenient, while the Telegram behavior fix itself is ready from this review's code and test evidence.

Validation

• git diff --check origin/master...HEAD — passed.
• cargo fmt --all -- --check — passed.
• cargo build --profile ci --locked --features channel-telegram — passed; Finished ci in 3m45s.
• cargo clippy --locked -p zeroclaw-channels --all-targets --features channel-telegram -- -D warnings — passed; Finished dev in 3m06s.
• cargo test --locked -p zeroclaw-channels --features channel-telegram --lib parse_update_reply_thread_shares_main_chat_history_key — passed: 1 passed.
• cargo test --locked -p zeroclaw-channels --features channel-telegram --lib attachment_parser_applies_the_topic_thread_gate — passed: 1 passed.
• cargo test --locked -p zeroclaw-channels --features channel-telegram --lib voice_parser_applies_the_topic_thread_gate — passed: 1 passed.
• cargo test --locked -p zeroclaw-channels --features channel-telegram --lib — passed: 1496 passed, 0 failed, 2 ignored.
• cargo nextest run --locked -p zeroclaw-channels --features channel-telegram --lib — passed: 1496 tests run, 1496 passed, 2 skipped.
• cargo build --profile ci --locked --no-default-features --features channel-telegram — passed; Finished ci in 4m19s.
• cargo clippy --locked -p zeroclaw-channels --no-default-features --all-targets --features channel-telegram -- -D warnings — passed; Finished dev in 6m09s.
• The first no-default-features test invocation was terminated by the 900-second command timeout while compiling under host-wide contention; it did not report a test failure. The rerun with CARGO_BUILD_JOBS=1, cargo test --locked -p zeroclaw-channels --no-default-features --features channel-telegram --lib -- --test-threads=1, passed: 878 passed, 0 failed, 2 ignored.
• ZEROCLAW_PARALLEL_TEST_SCOPE=channels ZEROCLAW_PARALLEL_TEST_RUNS=3 ZEROCLAW_PARALLEL_TEST_THREADS=16 ./scripts/ci/parallel_runtime_test_gate.sh — the local run stopped on its first pass with 1492 passed and 4 failures in unchanged Matrix typing timing tests (crates/zeroclaw-channels/src/orchestrator/mod.rs:16657, :22995, and :23025). No file under orchestrator/mod.rs is part of this PR. The exact-head GitHub Parallel Runtime Test check is successful.
• The initial combined test-name filter selected 0 tests because Cargo treats its argument as a substring rather than an alternation expression; each of the three added/changed tests was then rerun by its exact name above and passed. The zero-test run was not used as validation evidence.
• Fresh exact-head GitHub status is green: gh pr checks --required reports CI Required Gate pass; the PR status rollup also reports successful Test, Parallel Runtime Test, Lint, format, feature/default/no-default checks, Linux build, security, MSRV, and applicable platform checks. Path-filtered Docs Style, Nix, Web Permission, and targeted Windows Clippy checks are skipped and do not cover this Telegram-only Rust diff.
• A direct check of tiny_jpeg() with image::load_from_memory_with_format failed as described in the second warning; this is why the fixture is not treated as valid merely because file recognizes its JPEG header.
• No live Telegram chat smoke test was performed. The deterministic text, attachment, and voice parser paths are covered with local mocks; live Bot API delivery and ordinary-reply presentation remain outside this history-scope change.