Fix initialization, filter, and timestamp-contract issues (#99-#103)

[dceoy]

Summary

Fixes five issues found while debugging downstream live-trading consumers:

• initialize_and_login_mt5() reports last_error() read after shutdown(), masking the real login failure #100 — Mt5DataClient.initialize_and_login_mt5() now captures last_error() immediately after a failed initialize()/login() (before shutdown() closes the IPC connection), so the raised Mt5RuntimeError reports the real login/authorization error instead of the post-shutdown status.
• Mt5Client.__enter__/_initialize_if_needed silently ignore initialize() failure; *_total methods then return None despite -> int #101 — Mt5Client.__enter__() and _initialize_if_needed() now raise Mt5RuntimeError (with last_error() details) when initialize() returns False, matching Mt5DataClient behavior. The five *_total methods validate their responses with _validate_mt5_response_is_not_none, so they no longer return None despite -> int.
• Getters silently discard conflicting filters; history symbol filter matches substrings via *{symbol}* wildcard #102 — orders_get/positions_get/history_orders_get/history_deals_get raise ValueError when mutually exclusive filters are combined (previously they silently applied ticket > group > symbol precedence). The history symbol filter now post-filters rows on exact symbol equality after the *{symbol}* group-wildcard fetch, so broker-suffixed variants (EURUSD.m, EURUSD.raw) are no longer returned; combining symbol with group also raises instead of silently dropping group.
• area:data: document that MT5 epoch timestamps are broker-server-time and that converted datetimes are timezone-naive #99 (docs) — New "Timestamps and Timezones" sections in README.md, docs/index.md, and docs/api/utils.md, plus docstring notes: MT5 epochs are trade-server wall-clock labels (typically UTC+2/+3) and pdmt5's converted datetimes are timezone-naive server time, not UTC; date_from/date_to arguments are interpreted in server time by the MetaTrader5 API.
• Docs: reversed decorator order in utils.md and stale double-initialize/login examples in docs/api #103 (docs) — Fixed the reversed decorator order in docs/api/utils.md (set_index_if_possible must stay outermost), removed the redundant initialize()/login() calls inside with blocks across the docs (the context managers already handle both; the removed client.login(config.login, ...) examples also failed pyright strict), corrected the logging-level (INFO, not DEBUG) and conversion-default prose, and aligned Best Practices with the new raising behavior.

Intentional behavior changes

• with Mt5Client(...) and auto-initialization now raise Mt5RuntimeError on a dead terminal/connection instead of continuing silently; *_total methods raise instead of returning None.
• Conflicting filter combinations raise ValueError (the logging decorator lets ValueError propagate unwrapped); the history symbol filter is now an exact match.

Release

• Version bumped to 1.1.1

QA

• .agents/skills/local-qa/scripts/qa.sh — all checks passed
• uv run pytest — 421 passed, 33 skipped (Windows-only), 100% branch coverage
• uv run ruff check . / uv run pyright — clean
• uv run mkdocs build — succeeds

Windows/MT5 note: all changes are exercised via the existing mock-based test suite; no live-terminal behavior was verified (CI runs on Windows).

Closes #99
Closes #100
Closes #101
Closes #102
Closes #103

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8fd232a9b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

[Review tooling noise — superseded by the final review below.]

[github-actions]

[Review tooling noise — superseded by the final review below.]

[github-actions]

[Review tooling noise — superseded by the final review below.]

[github-actions]

Reviewed code quality, performance, tests, documentation, security, types, and silent-failure concerns. Overall this is a solid, focused set of fixes. A few important issues to address before merging:

Implementation issues

• pdmt5/mt5.py:73 — _log_mt5_last_status_code still wraps internally-raised Mt5RuntimeError into a generic message (see inline comment).
• pdmt5/mt5.py:1073 — _validate_history_filters allows group without a date range, contradicting its docstring.
• pdmt5/dataframe.py:1126 — _validate_history_input does not reject symbol/group combined with ticket/position, producing a confusing downstream error.

Documentation

• docs/api/mt5.md:221 — the claim that all methods raise Mt5RuntimeError is too broad.
• Many pdmt5/mt5.py docstrings still say "or None" for methods that now raise on None: account_info, terminal_info, symbols_get, symbol_info, symbol_info_tick, market_book_get, copy_rates_from, copy_rates_from_pos, copy_rates_range, copy_ticks_from, copy_ticks_range, orders_get, order_calc_margin, order_calc_profit, order_check, order_send, positions_get, history_orders_get, history_deals_get.
• docs/api/utils.md:66 — the millisecond rule says any field ending in _msc is converted, but the code only converts keys that start with time_ and end with _msc.

Test gaps

• tests/test_dataframe.py lacks coverage for Mt5DataClient.__enter__ raising on initialization failure and for conflicting filters on the orders_get_as_df/positions_get_as_df path.

Suggestions

• Narrow **filters: Any in _validate_filters_are_exclusive to str | int | None.
• Update one-line summaries in orders_get/positions_get/history_orders_get/history_deals_get to mention group.
• Consider filtering the raw MT5 sequence before dict conversion to reduce allocation for large histories.

None of these block merge, but the docstring/implementation inconsistencies and the Mt5RuntimeError wrapping are worth fixing.

[opencode-agent]

Posted a review on PR #104: #104 (review)

I ran the full review agent set (code quality, performance, tests, documentation, security, code guidelines, types, comments, and silent-failure). The review is a COMMENT (not blocking) with six inline comments and a summary body.

Key findings noted:

• _log_mt5_last_status_code still wraps internally-raised Mt5RuntimeError into a generic message.
• _validate_history_filters allows group without a date range, contradicting its docstring.
• _validate_history_input doesn't reject symbol/group combined with ticket/position, leading to confusing downstream errors.
• Several docstrings still say "or None" for methods that now raise on None.
• docs/api/mt5.md overstates that all methods raise Mt5RuntimeError.
• A few test gaps remain in the DataFrame wrapper paths.

Note: uv was not available in this environment, so I couldn't run .agents/skills/local-qa/scripts/qa.sh locally; the PR body already reports all checks passed.

github run

[dceoy]

Reviewed the MT5 initialization/error-path changes, mutually-exclusive filter validation, exact symbol post-filtering, timestamp/timezone documentation updates, and the reported CI status. I do not see a blocking issue in this diff. The behavior changes appear covered by the expanded mock-based tests, and the latest CI/CD run is green.

[dceoy]

Reviewed the implementation, tests, and docs for PR #104.

No additional blocking findings.

What I checked:

• Initialization failure now raises Mt5RuntimeError through __enter__() / auto-initialization, and tests cover both direct context-manager and lazy-initialization paths.
• *_total methods now validate None responses while preserving valid zero counts.
• orders_get / positions_get / history getters now reject ambiguous filter combinations before delegating to MT5.
• History symbol filtering now fetches via the required MT5 wildcard group and post-filters exact symbol matches, with tests covering broker-suffixed variants.
• Decorator ordering for datetime conversion before index setting is consistent with the documented behavior.
• Docs have been updated to align with the intentional breaking changes and timestamp contract.
• CI for head commit 0801b2972a62740caa81a55287ba51cb84c37b21 is green.

One non-blocking note: the timestamp/server-time contract intentionally differs from the literal wording in the official MQL5 docs. The PR documents that nuance clearly enough for now, but downstream live-terminal validation remains valuable before relying on it as a hard public contract.