Fix check_win: Map server deal IDs to client request IDs

[Copilot]

check_win never finds completed orders because the server uses its own deal id in completion messages, while orders are stored under the client-generated requestId.

Changes

• Added _server_id_to_request_id mapping dict — Captures server ID → request ID relationship when order confirmation arrives with both fields
• Updated deal processing — Looks up original requestId via mapping before searching _active_orders
• Store results under requestId — Ensures check_win and check_order_result find completed orders
• Cleanup mapping on completion — Prevents memory leak

Flow Before

Client places order with requestId="abc-123"
Server confirms with id="98765", stored under "abc-123"
Server completes deal with id="98765"
Code looks for "98765" in _active_orders → not found ❌

Flow After

Server confirms with id="98765" → maps "98765" → "abc-123"
Server completes deal with id="98765"
Code looks up mapping → finds "abc-123" → retrieves order ✓

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Check win not working</issue_title>
<issue_description>Pls fix check_win</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Check win not working #47

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR fixes a critical bug in the check_win functionality where completed orders were never found because the server uses its own deal IDs in completion messages while orders were stored under client-generated request IDs. The fix introduces a mapping mechanism to translate between server IDs and client request IDs.

• Adds a bidirectional mapping system to correlate server-assigned deal IDs with client request IDs
• Updates the deal processing logic to use the mapping for order lookups
• Includes comprehensive test coverage for the mapping functionality

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
pocketoptionapi_async/client.py	Adds _server_id_to_request_id mapping dict, updates order confirmation to store mappings, modifies deal completion to lookup via mapping, and cleans up mappings after use
tests/test_check_win.py	New comprehensive test file validating the ID mapping fix with win/loss scenarios and backward compatibility

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Redundant check: server_id is guaranteed to be non-empty at this point. Line 1040 already checks data['id'] is truthy before converting to string on line 1041. An empty string input would have failed the line 1040 check. This redundant check on line 1042 adds unnecessary complexity.

Suggested change

	if server_id: # Ensure string is not empty
	self._server_id_to_request_id[server_id] = request_id
	if self.enable_logging:
	logger.debug(f"Mapped server ID {server_id} to request ID {request_id}")
	self._server_id_to_request_id[server_id] = request_id
	if self.enable_logging:
	logger.debug(f"Mapped server ID {server_id} to request ID {request_id}")

[Copilot]

Incorrect short-circuit evaluation order. The current order await test() and all_passed will skip tests if all_passed is already False due to short-circuiting. The tests should run first and then AND with all_passed. Change to all_passed = all_passed and await test() to ensure all tests execute.

Suggested change

	all_passed = await test_check_win_id_mapping() and all_passed
	except Exception as e:
	print(f"❌ FAIL: test_check_win_id_mapping - {e}")
	all_passed = False
	try:
	all_passed = await test_check_win_loss_scenario() and all_passed
	except Exception as e:
	print(f"❌ FAIL: test_check_win_loss_scenario - {e}")
	all_passed = False
	try:
	all_passed = await test_check_win_fallback_without_mapping() and all_passed
	all_passed = all_passed and await test_check_win_id_mapping()
	except Exception as e:
	print(f"❌ FAIL: test_check_win_id_mapping - {e}")
	all_passed = False
	try:
	all_passed = all_passed and await test_check_win_loss_scenario()
	except Exception as e:
	print(f"❌ FAIL: test_check_win_loss_scenario - {e}")
	all_passed = False
	try:
	all_passed = all_passed and await test_check_win_fallback_without_mapping()

[Copilot]

Incorrect short-circuit evaluation order. The current order await test() and all_passed will skip tests if all_passed is already False due to short-circuiting. The tests should run first and then AND with all_passed. Change to all_passed = all_passed and await test() to ensure all tests execute.

Suggested change

	all_passed = await test_check_win_id_mapping() and all_passed
	except Exception as e:
	print(f"❌ FAIL: test_check_win_id_mapping - {e}")
	all_passed = False
	try:
	all_passed = await test_check_win_loss_scenario() and all_passed
	except Exception as e:
	print(f"❌ FAIL: test_check_win_loss_scenario - {e}")
	all_passed = False
	try:
	all_passed = await test_check_win_fallback_without_mapping() and all_passed
	all_passed = all_passed and await test_check_win_id_mapping()
	except Exception as e:
	print(f"❌ FAIL: test_check_win_id_mapping - {e}")
	all_passed = False
	try:
	all_passed = all_passed and await test_check_win_loss_scenario()
	except Exception as e:
	print(f"❌ FAIL: test_check_win_loss_scenario - {e}")
	all_passed = False
	try:
	all_passed = all_passed and await test_check_win_fallback_without_mapping()

[Copilot]

Incorrect short-circuit evaluation order. The current order await test() and all_passed will skip tests if all_passed is already False due to short-circuiting. The tests should run first and then AND with all_passed. Change to all_passed = all_passed and await test() to ensure all tests execute.

Suggested change

	all_passed = await test_check_win_id_mapping() and all_passed
	except Exception as e:
	print(f"❌ FAIL: test_check_win_id_mapping - {e}")
	all_passed = False
	try:
	all_passed = await test_check_win_loss_scenario() and all_passed
	except Exception as e:
	print(f"❌ FAIL: test_check_win_loss_scenario - {e}")
	all_passed = False
	try:
	all_passed = await test_check_win_fallback_without_mapping() and all_passed
	all_passed = all_passed and await test_check_win_id_mapping()
	except Exception as e:
	print(f"❌ FAIL: test_check_win_id_mapping - {e}")
	all_passed = False
	try:
	all_passed = all_passed and await test_check_win_loss_scenario()
	except Exception as e:
	print(f"❌ FAIL: test_check_win_loss_scenario - {e}")
	all_passed = False
	try:
	all_passed = all_passed and await test_check_win_fallback_without_mapping()

[Copilot]

[nitpick] The comment on line 1086-1087 could be clearer. Consider: 'Use mapped request_id if available; otherwise fall back to server_deal_id for backward compatibility' to explicitly mention backward compatibility, which is the key design decision here.

Suggested change

	# If we have a mapping, use request_id to find the order
	# Otherwise, fall back to trying server_deal_id directly
	# Use mapped request_id if available; otherwise fall back to server_deal_id for backward compatibility