Open
Conversation
Review Summary by QodoImplement Dashboard API client with dual data source strategy
WalkthroughsDescription• Implement Dashboard API client with dual data source strategy - API-first approach with HTML fallback for reliability - Retry mechanism (2 retries, 1-second delay) and timeout handling - Session expiration detection (401/403) triggers automatic fallback • Integrate DashboardClient into PointsDetector for points retrieval - Prioritize API calls over HTML parsing - Graceful error handling with specific exception types • Add comprehensive data models for Dashboard API responses - Dataclass-based models with camelCase to snake_case conversion - Support for user status, search counters, promotions, and punch cards • Comprehensive test coverage with 13 unit tests - Mock API responses, retry logic, timeout, and network error scenarios - HTML fallback validation for all failure cases Diagramflowchart LR
A["PointsDetector"] -->|"try API first"| B["DashboardClient"]
B -->|"success"| C["Return Points"]
B -->|"failure/timeout"| D["HTML Fallback"]
D -->|"success"| C
D -->|"failure"| E["DashboardError"]
B -->|"401/403"| D
F["API Models"] -->|"parse response"| B
File Changes1. src/api/__init__.py
|
Code Review by Qodo
1. List items unchecked
|
Disaster-Terminator
force-pushed
the
feature/dashboard-api
branch
from
629d79c to
fe110c2
Compare
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
该 PR 引入 Dashboard API 客户端，优先通过 Rewards Dashboard API 拉取积分/计数器数据，并在失败时回退到 HTML 解析，以提升获取积分数据的稳定性与性能。
Changes:
- 新增
src/api/模块：DashboardClient + 数据模型（dataclass）封装 API/HTML 数据解析 - 在
PointsDetector.get_current_points中集成 API 优先读取积分，失败回退到现有 HTML 逻辑 - 新增单元测试并引入
respx作为 HTTPX mock 依赖
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_dashboard_client.py | 增加 DashboardClient 与模型解析的单测覆盖（含重试/fallback） |
| src/api/models.py | 定义 dashboard 响应的 dataclass 模型与 from_dict 解析逻辑 |
| src/api/dashboard_client.py | 实现 API 调用 + 重试 + 401/403 处理 + HTML fallback 的客户端 |
| src/api/init.py | 暴露 API 模块公共导出 |
| src/account/points_detector.py | 积分检测逻辑改为 API 优先，HTML 作为备用 |
| pyproject.toml | 新增 respx 测试依赖 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reviewer's Guide在新的 Dashboard API 模型和客户端的类图classDiagram
class DashboardError {
+int status_code
+__init__(message: str, status_code: int)
+is_auth_error() bool
}
class DashboardClient {
-Page _page
-int _max_retries
-float _retry_delay
-float _timeout
-AsyncClient _client
-str _api_url
+DEFAULT_MAX_RETRIES: int
+DEFAULT_RETRY_DELAY: float
+DEFAULT_TIMEOUT: float
+__init__(page: Page, max_retries: int, retry_delay: float, timeout: float)
+close() None
+__aenter__() DashboardClient
+__aexit__(exc_type: type, exc_val: BaseException, exc_tb: object) None
-_get_cookies_header() str
-_call_api() DashboardData
-_parse_dashboard(data: dict~str, object~) DashboardData
-_html_fallback() DashboardData
+get_dashboard_data() DashboardData
+get_current_points() int
+get_search_counters() SearchCounters
}
class DashboardData {
+UserStatus user_status
+dict~str, list~Promotion~~ daily_set_promotions
+list~Promotion~ more_promotions
+list~PunchCard~ punch_cards
+StreakPromotion streak_promotion
+list~StreakBonusPromotion~ streak_bonus_promotions
+from_dict(data: dict~str, Any~) DashboardData
}
class UserStatus {
+int available_points
+LevelInfo level_info
+SearchCounters counters
+int bing_star_monthly_bonus_progress
+int bing_star_monthly_bonus_maximum
+int default_search_engine_monthly_bonus_progress
+int default_search_engine_monthly_bonus_maximum
+str default_search_engine_monthly_bonus_state
+from_dict(data: dict~str, Any~) UserStatus
}
class LevelInfo {
+str active_level
+str active_level_name
+int progress
+int progress_max
+from_dict(data: dict~str, Any~) LevelInfo
}
class SearchCounter {
+int progress
+int max_progress
+from_dict(data: dict~str, Any~) SearchCounter
}
class SearchCounters {
+list~SearchCounter~ pc_search
+list~SearchCounter~ mobile_search
+from_dict(data: dict~str, Any~) SearchCounters
}
class Promotion {
+str promotion_type
+str title
+int points
+str status
+str url
+from_dict(data: dict~str, Any~) Promotion
}
class PunchCard {
+str name
+int progress
+int max_progress
+bool completed
+from_dict(data: dict~str, Any~) PunchCard
}
class StreakPromotion {
+str promotion_type
+str title
+int points
+str status
+str url
+int streak_count
+from_dict(data: dict~str, Any~) StreakPromotion
}
class StreakBonusPromotion {
+str promotion_type
+str title
+int points
+str status
+str url
+int streak_day
+from_dict(data: dict~str, Any~) StreakBonusPromotion
}
class PointsDetector {
+get_current_points(page: Page, skip_navigation: bool) int
-_check_task_status(page: Page, selectors: list, task_name: str) dict~str, object~
-_extract_points_from_source(page: Page) int
-_parse_points(points_text: str) int
}
DashboardClient --> DashboardError : raises
DashboardClient --> DashboardData : returns
DashboardClient --> SearchCounters : returns
DashboardData --> UserStatus
DashboardData --> Promotion
DashboardData --> PunchCard
DashboardData --> StreakPromotion
DashboardData --> StreakBonusPromotion
UserStatus --> LevelInfo
UserStatus --> SearchCounters
SearchCounters --> SearchCounter
PointsDetector --> DashboardClient : uses
PointsDetector ..> DashboardError : handles
文件级变更
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideIntroduces a typed, reusable Dashboard API client and data models under a new api package, wires it into the points detector with an API‑first, HTML‑fallback strategy, tightens task status parsing, and adds httpx/respx plus tests and configuration updates needed to support the new integration. Class diagram for Dashboard API models and clientclassDiagram
class DashboardError {
+int status_code
+__init__(message: str, status_code: int)
+is_auth_error() bool
}
class DashboardClient {
-Page _page
-int _max_retries
-float _retry_delay
-float _timeout
-AsyncClient _client
-str _api_url
+DEFAULT_MAX_RETRIES: int
+DEFAULT_RETRY_DELAY: float
+DEFAULT_TIMEOUT: float
+__init__(page: Page, max_retries: int, retry_delay: float, timeout: float)
+close() None
+__aenter__() DashboardClient
+__aexit__(exc_type: type, exc_val: BaseException, exc_tb: object) None
-_get_cookies_header() str
-_call_api() DashboardData
-_parse_dashboard(data: dict~str, object~) DashboardData
-_html_fallback() DashboardData
+get_dashboard_data() DashboardData
+get_current_points() int
+get_search_counters() SearchCounters
}
class DashboardData {
+UserStatus user_status
+dict~str, list~Promotion~~ daily_set_promotions
+list~Promotion~ more_promotions
+list~PunchCard~ punch_cards
+StreakPromotion streak_promotion
+list~StreakBonusPromotion~ streak_bonus_promotions
+from_dict(data: dict~str, Any~) DashboardData
}
class UserStatus {
+int available_points
+LevelInfo level_info
+SearchCounters counters
+int bing_star_monthly_bonus_progress
+int bing_star_monthly_bonus_maximum
+int default_search_engine_monthly_bonus_progress
+int default_search_engine_monthly_bonus_maximum
+str default_search_engine_monthly_bonus_state
+from_dict(data: dict~str, Any~) UserStatus
}
class LevelInfo {
+str active_level
+str active_level_name
+int progress
+int progress_max
+from_dict(data: dict~str, Any~) LevelInfo
}
class SearchCounter {
+int progress
+int max_progress
+from_dict(data: dict~str, Any~) SearchCounter
}
class SearchCounters {
+list~SearchCounter~ pc_search
+list~SearchCounter~ mobile_search
+from_dict(data: dict~str, Any~) SearchCounters
}
class Promotion {
+str promotion_type
+str title
+int points
+str status
+str url
+from_dict(data: dict~str, Any~) Promotion
}
class PunchCard {
+str name
+int progress
+int max_progress
+bool completed
+from_dict(data: dict~str, Any~) PunchCard
}
class StreakPromotion {
+str promotion_type
+str title
+int points
+str status
+str url
+int streak_count
+from_dict(data: dict~str, Any~) StreakPromotion
}
class StreakBonusPromotion {
+str promotion_type
+str title
+int points
+str status
+str url
+int streak_day
+from_dict(data: dict~str, Any~) StreakBonusPromotion
}
class PointsDetector {
+get_current_points(page: Page, skip_navigation: bool) int
-_check_task_status(page: Page, selectors: list, task_name: str) dict~str, object~
-_extract_points_from_source(page: Page) int
-_parse_points(points_text: str) int
}
DashboardClient --> DashboardError : raises
DashboardClient --> DashboardData : returns
DashboardClient --> SearchCounters : returns
DashboardData --> UserStatus
DashboardData --> Promotion
DashboardData --> PunchCard
DashboardData --> StreakPromotion
DashboardData --> StreakBonusPromotion
UserStatus --> LevelInfo
UserStatus --> SearchCounters
SearchCounters --> SearchCounter
PointsDetector --> DashboardClient : uses
PointsDetector ..> DashboardError : handles
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并给出了一些高层次的反馈：
- 在
DashboardClient.get_dashboard_data中，通过字符串匹配（在str(e)中查找"HTTP error: 401"/"403"）来检测认证失败比较脆弱；建议从_call_api传递状态码（例如通过自定义异常属性或子类），然后基于数值状态码进行分支判断。 - HTML 回退使用的正则
r"var\s+dashboard\s*=\s*({.*?});"会在遇到第一个右大括号时停止，如果 dashboard 对象中包含嵌套的大括号或内联脚本，就可能解析失败；建议使用更健壮的提取方式（例如匹配到结束的;</script>，或者使用支持括号配对的解析器）以减少解析失败。 - 若干
from_dict构造器（例如SearchCounter.from_dict、LevelInfo.from_dict、Promotion.from_dict）在_transform_dict之后直接调用cls(**data)，一旦 API 添加了未预期的字段就会抛异常；建议通过显式选择已知字段（例如使用get或按字段白名单过滤）来构造实例，使这些模型对 schema 变更更具弹性。
给 AI Agent 的提示词
请根据以下代码评审意见进行修改：
## 总体意见
- 在 `DashboardClient.get_dashboard_data` 中，通过字符串匹配（在 `str(e)` 中查找 `"HTTP error: 401"` / `"403"`）来检测认证失败比较脆弱；建议从 `_call_api` 传递状态码（例如通过自定义异常属性或子类），然后基于数值状态码进行分支判断。
- HTML 回退使用的正则 `r"var\s+dashboard\s*=\s*({.*?});"` 会在遇到第一个右大括号时停止，如果 dashboard 对象中包含嵌套的大括号或内联脚本，就可能解析失败；建议使用更健壮的提取方式（例如匹配到结束的 `;</script>`，或者使用支持括号配对的解析器）以减少解析失败。
- 若干 `from_dict` 构造器（例如 `SearchCounter.from_dict`、`LevelInfo.from_dict`、`Promotion.from_dict`）在 `_transform_dict` 之后直接调用 `cls(**data)`，一旦 API 添加了未预期的字段就会抛异常；建议通过显式选择已知字段（例如使用 `get` 或按字段白名单过滤）来构造实例，使这些模型对 schema 变更更具弹性。
## 单独评论
### 评论 1
<location path="src/api/dashboard_client.py" line_range="70" />
<code_context>
+ async with httpx.AsyncClient(timeout=self.TIMEOUT) as client:
+ response = await client.get(self.API_URL, headers=headers)
+ response.raise_for_status()
+ data = response.json()
+
+ if "dashboard" not in data:
</code_context>
<issue_to_address>
**issue:** 来自 API 响应的 JSON 解析错误没有被包装为 DashboardError，可能会泄漏出非预期的异常。
如果 `response.json()` 失败（例如返回 200 状态但 body 是 HTML，或者 JSON 格式损坏），会抛出 `JSONDecodeError`，并跳过 `DashboardError` 的处理路径，从而导致重试/回退逻辑无法执行。请用 try/except 包裹 `response.json()` 并重新抛出为 `DashboardError`，以保持错误处理和重试逻辑的一致性。
</issue_to_address>
### 评论 2
<location path="src/api/dashboard_client.py" line_range="69-75" />
<code_context>
+ response.raise_for_status()
+ data = response.json()
+
+ if "dashboard" not in data:
+ raise DashboardError("Invalid API response: missing 'dashboard' field")
+
+ return self._parse_dashboard(data["dashboard"])
</code_context>
<issue_to_address>
**suggestion:** 对 `"dashboard"` 键的检查假设响应一定是字典，如果 JSON 负载不是字典可能会出现异常行为。
如果 API 返回的是非对象类型 JSON（例如 list 或 null），那么执行 `"dashboard" not in data` 会抛出 `TypeError`，而不是 `DashboardError`，从而跳过预期的重试/回退逻辑。建议先用 `isinstance(data, dict)` 做类型判断，如果不是字典，则抛出 `DashboardError`，以保证失败场景都能被一致地处理。
```suggestion
response.raise_for_status()
data = response.json()
if not isinstance(data, dict) or "dashboard" not in data:
raise DashboardError("Invalid API response: expected JSON object with 'dashboard' field")
return self._parse_dashboard(data["dashboard"])
```
</issue_to_address>
### 评论 3
<location path="src/api/models.py" line_range="34-35" />
<code_context>
+ @classmethod
+ def from_dict(cls, data: dict[str, Any]) -> "SearchCounter":
+ """从字典创建实例"""
+ data = _transform_dict(data)
+ return cls(**data)
+
+
</code_context>
<issue_to_address>
**suggestion:** 直接使用 `cls(**data)` 会让模型对 API schema 变更和额外字段非常脆弱。
由于这里是在解析外部 API 的返回，一旦服务端新增了不在 dataclass 中定义的字段，`cls(**data)` 就会抛出 `TypeError`，导致客户端崩溃。请在实例化前先过滤 `data`，只保留存在于 `cls.__dataclass_fields__` 中的键，同时对其他同样通过 `**data` 解包调用数据类构造函数的 `from_dict` 方法也做相同处理。
建议实现如下：
```python
@dataclass
class SearchCounter:
"""搜索计数器"""
progress: int = 0
max_progress: int = 0
@classmethod
def from_dict(cls, data: dict[str, Any]) -> "SearchCounter":
"""从字典创建实例"""
# 先进行统一的字段转换（例如 camelCase -> snake_case）
data = _transform_dict(data)
# 只保留当前数据类中定义的字段，避免外部 API 新增字段导致 TypeError
allowed_fields = cls.__dataclass_fields__.keys()
filtered_data = {k: v for k, v in data.items() if k in allowed_fields}
return cls(**filtered_data)
```
在 `src/api/models.py`（或相关的模型文件）中，可能还有其他 `from_dict` 类方法同样是先执行 `data = _transform_dict(data)`，然后直接 `cls(**data)`。
对于这些方法，请依次应用同样的模式：
1. 在 `_transform_dict(data)` 之后，计算 `allowed_fields = cls.__dataclass_fields__.keys()`。
2. 构造 `filtered_data = {k: v for k, v in data.items() if k in allowed_fields}`。
3. 使用 `return cls(**filtered_data)` 实例化，而不是 `cls(**data)`。
这样可以确保所有 dataclass 模型在面对外部 API 的 schema 变更时依然健壮，并且可以安全地忽略未知字段。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据反馈改进后续评审。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
DashboardClient.get_dashboard_data, relying on string matching ("HTTP error: 401"/"403"instr(e)) to detect auth failures is brittle; consider propagating the status code from_call_api(e.g., via a custom exception attribute or subclass) and branching on the numeric code instead. - The HTML fallback regex
r"var\s+dashboard\s*=\s*({.*?});"will stop at the first closing brace and can break if the dashboard object contains nested braces or embedded scripts; consider a more robust extraction (e.g., matching until the terminating;</script>or using a balanced-brace parser) to reduce parse failures. - Several
from_dictconstructors (e.g.,SearchCounter.from_dict,LevelInfo.from_dict,Promotion.from_dict) callcls(**data)after_transform_dict, which will raise if the API adds unexpected fields; making these more defensive by explicitly picking known keys (e.g., viaget) would make the models more resilient to schema changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `DashboardClient.get_dashboard_data`, relying on string matching (`"HTTP error: 401"` / `"403"` in `str(e)`) to detect auth failures is brittle; consider propagating the status code from `_call_api` (e.g., via a custom exception attribute or subclass) and branching on the numeric code instead.
- The HTML fallback regex `r"var\s+dashboard\s*=\s*({.*?});"` will stop at the first closing brace and can break if the dashboard object contains nested braces or embedded scripts; consider a more robust extraction (e.g., matching until the terminating `;</script>` or using a balanced-brace parser) to reduce parse failures.
- Several `from_dict` constructors (e.g., `SearchCounter.from_dict`, `LevelInfo.from_dict`, `Promotion.from_dict`) call `cls(**data)` after `_transform_dict`, which will raise if the API adds unexpected fields; making these more defensive by explicitly picking known keys (e.g., via `get`) would make the models more resilient to schema changes.
## Individual Comments
### Comment 1
<location path="src/api/dashboard_client.py" line_range="70" />
<code_context>
+ async with httpx.AsyncClient(timeout=self.TIMEOUT) as client:
+ response = await client.get(self.API_URL, headers=headers)
+ response.raise_for_status()
+ data = response.json()
+
+ if "dashboard" not in data:
</code_context>
<issue_to_address>
**issue:** JSON parsing errors from the API response are not wrapped in DashboardError, which can leak unexpected exceptions.
If `response.json()` fails (e.g., HTML error body with 200 or malformed JSON), it will raise `JSONDecodeError` and skip the `DashboardError` path, so retries/fallbacks won’t run. Please wrap `response.json()` in try/except and re-raise as `DashboardError` to keep error handling and retry logic consistent.
</issue_to_address>
### Comment 2
<location path="src/api/dashboard_client.py" line_range="69-75" />
<code_context>
+ response.raise_for_status()
+ data = response.json()
+
+ if "dashboard" not in data:
+ raise DashboardError("Invalid API response: missing 'dashboard' field")
+
+ return self._parse_dashboard(data["dashboard"])
</code_context>
<issue_to_address>
**suggestion:** The `"dashboard"` key check assumes a dict response and may misbehave if the JSON payload is not a dict.
If the API ever returns a non-object JSON (e.g., list or null), the `"dashboard" not in data` check will raise a `TypeError` instead of a `DashboardError`, skipping the intended retry/fallback path. Consider guarding with `isinstance(data, dict)` and raising `DashboardError` when it’s not, so failures are handled consistently.
```suggestion
response.raise_for_status()
data = response.json()
if not isinstance(data, dict) or "dashboard" not in data:
raise DashboardError("Invalid API response: expected JSON object with 'dashboard' field")
return self._parse_dashboard(data["dashboard"])
```
</issue_to_address>
### Comment 3
<location path="src/api/models.py" line_range="34-35" />
<code_context>
+ @classmethod
+ def from_dict(cls, data: dict[str, Any]) -> "SearchCounter":
+ """从字典创建实例"""
+ data = _transform_dict(data)
+ return cls(**data)
+
+
</code_context>
<issue_to_address>
**suggestion:** Using `cls(**data)` directly makes the models brittle to API schema changes and extra fields.
Since this parses responses from an external API, any new field added by the server that isn’t in the dataclass will raise a `TypeError` from `cls(**data)` and break the client. Please filter `data` to only include keys present in `cls.__dataclass_fields__` before instantiating, and apply the same approach to other `from_dict` methods that unpack `**data` into dataclass constructors.
Suggested implementation:
```python
@dataclass
class SearchCounter:
"""搜索计数器"""
progress: int = 0
max_progress: int = 0
@classmethod
def from_dict(cls, data: dict[str, Any]) -> "SearchCounter":
"""从字典创建实例"""
# 先进行统一的字段转换（例如 camelCase -> snake_case）
data = _transform_dict(data)
# 只保留当前数据类中定义的字段，避免外部 API 新增字段导致 TypeError
allowed_fields = cls.__dataclass_fields__.keys()
filtered_data = {k: v for k, v in data.items() if k in allowed_fields}
return cls(**filtered_data)
```
There may be other `from_dict` classmethods in `src/api/models.py` (or related model files) that similarly do `data = _transform_dict(data)` followed by `cls(**data)`.
For each of those, the same pattern should be applied:
1. After `_transform_dict(data)`, compute `allowed_fields = cls.__dataclass_fields__.keys()`.
2. Build `filtered_data = {k: v for k, v in data.items() if k in allowed_fields}`.
3. Instantiate with `return cls(**filtered_data)` instead of `cls(**data)`.
This ensures all dataclass models are resilient to external API schema changes and ignore unknown fields safely.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Disaster-Terminator
force-pushed
the
feature/dashboard-api
branch
4 times, most recently
from
f46d43b to
073fc9d
Compare
Owner
Author
|
@sourcery-ai review |
Owner
Author
|
/agentic_review |
|
Persistent review updated to latest commit 073fc9d |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SourceryAI
reviewed
Feb 26, 2026
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并留下了一些高层反馈：
- 在
DashboardClient._call_api中你把重试次数写死为range(2)，而且这套重试逻辑和get_dashboard_data中的重试逻辑是分离的；后者已经基于self._max_retries处理重试。建议把这些逻辑统一起来，让所有重试行为和重试间隔都由可配置的max_retries/retry_delay驱动，以避免令人意外的行为和重复逻辑。 - HTML 兜底的正则
re.search(r"var\s+dashboard\s*=\s*({.*?});", html, re.DOTALL)对 JS 的具体形态要求比较严格；你可能需要让它更健壮一些（例如容忍let/const、可选分号，或额外的空白/注释），从而在页面脚本格式略有变化时减少 fragility。 - 在
api.models中，_camel_to_snake在函数内部导入re，并被其他构造器间接使用；将re的导入移动到模块级可以避免在大量构造对象的热点路径上重复导入。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `DashboardClient._call_api` you hard-code `range(2)` and have separate retry logic from `get_dashboard_data`, which already handles retries based on `self._max_retries`; consider unifying these so all retry behavior and delays are driven by the configurable `max_retries`/`retry_delay` to avoid surprising behavior and duplicated logic.
- The HTML fallback regex `re.search(r"var\s+dashboard\s*=\s*({.*?});", html, re.DOTALL)` is quite strict about the exact JS shape; you might want to make this more robust (e.g., tolerate `let/const`, optional semicolon, or additional whitespace/comments) to reduce fragility if the page script format changes slightly.
- In `api.models`, `_camel_to_snake` imports `re` inside the function and is used transitively by other constructors; moving the `re` import to the module level will avoid repeated imports on hot paths where many objects are constructed.
## Individual Comments
### Comment 1
<location path="src/api/dashboard_client.py" line_range="113-114" />
<code_context>
+ "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36",
+ }
+
+ last_error: Exception | None = None
+ for attempt in range(2):
+ try:
+ client = await self._get_client()
</code_context>
<issue_to_address>
**suggestion:** Use the configurable retry count instead of the hard-coded `range(2)` in `_call_api`.
`DEFAULT_MAX_RETRIES` and the `max_retries` override aren’t honored here because `_call_api` uses `range(2)`. Please use the configured retry value (e.g., `range(self._max_retries + 1)`) so caller configuration is respected and behavior is consistent.
```suggestion
last_error: Exception | None = None
for attempt in range(self._max_retries + 1):
```
</issue_to_address>
### Comment 2
<location path="src/api/models.py" line_range="9-13" />
<code_context>
+_T = TypeVar("_T")
+
+
+def _camel_to_snake(name: str) -> str:
+ """将 camelCase 转换为 snake_case"""
+ import re
+
+ return re.sub(r"([a-z0-9])([A-Z])", r"\1_\2", name).lower()
+
+
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid importing `re` inside `_camel_to_snake` on every call.
Because `_camel_to_snake` may be called frequently via `_transform_dict`, importing `re` on each call adds avoidable overhead. Move `import re` to module scope and, if needed, precompile the regex with `re.compile` to keep this hot path efficient and clear.
</issue_to_address>
### Comment 3
<location path="src/api/models.py" line_range="39-48" />
<code_context>
+ progress: int = 0
+ max_progress: int = 0
+
+ @classmethod
+ def from_dict(cls, data: dict[str, Any]) -> "SearchCounter":
+ """从字典创建实例"""
+ data = _transform_dict(data)
+ return cls(**_filter_dataclass_fields(data, cls))
+
+
+@dataclass
+class SearchCounters:
+ """搜索计数器集合"""
+
+ pc_search: list[SearchCounter] = field(default_factory=list)
+ mobile_search: list[SearchCounter] = field(default_factory=list)
+
+ @classmethod
+ def from_dict(cls, data: dict[str, Any]) -> "SearchCounters":
+ """从字典创建实例"""
+ data = _transform_dict(data)
</code_context>
<issue_to_address>
**suggestion:** Handle non-list values for `pc_search`/`mobile_search` more defensively in `SearchCounters.from_dict`.
This currently assumes `pc_search` and `mobile_search` are always iterable lists. If the API returns `null` or a scalar, iteration will fail. Consider normalizing these keys first, e.g.:
```python
pc_raw = data.get("pc_search") or []
if not isinstance(pc_raw, list):
pc_raw = []
```
Apply similarly for `mobile_search` so `from_dict` is resilient to schema drift while keeping the same public API.
</issue_to_address>Hi @Disaster-Terminator! 👋
感谢你通过评论 @sourcery-ai review 来试用 Sourcery！🚀
安装 sourcery-ai bot 来在每个 Pull Request 上获取自动代码审查 ✨
帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续的审查。Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
DashboardClient._call_apiyou hard-coderange(2)and have separate retry logic fromget_dashboard_data, which already handles retries based onself._max_retries; consider unifying these so all retry behavior and delays are driven by the configurablemax_retries/retry_delayto avoid surprising behavior and duplicated logic. - The HTML fallback regex
re.search(r"var\s+dashboard\s*=\s*({.*?});", html, re.DOTALL)is quite strict about the exact JS shape; you might want to make this more robust (e.g., toleratelet/const, optional semicolon, or additional whitespace/comments) to reduce fragility if the page script format changes slightly. - In
api.models,_camel_to_snakeimportsreinside the function and is used transitively by other constructors; moving thereimport to the module level will avoid repeated imports on hot paths where many objects are constructed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `DashboardClient._call_api` you hard-code `range(2)` and have separate retry logic from `get_dashboard_data`, which already handles retries based on `self._max_retries`; consider unifying these so all retry behavior and delays are driven by the configurable `max_retries`/`retry_delay` to avoid surprising behavior and duplicated logic.
- The HTML fallback regex `re.search(r"var\s+dashboard\s*=\s*({.*?});", html, re.DOTALL)` is quite strict about the exact JS shape; you might want to make this more robust (e.g., tolerate `let/const`, optional semicolon, or additional whitespace/comments) to reduce fragility if the page script format changes slightly.
- In `api.models`, `_camel_to_snake` imports `re` inside the function and is used transitively by other constructors; moving the `re` import to the module level will avoid repeated imports on hot paths where many objects are constructed.
## Individual Comments
### Comment 1
<location path="src/api/dashboard_client.py" line_range="113-114" />
<code_context>
+ "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36",
+ }
+
+ last_error: Exception | None = None
+ for attempt in range(2):
+ try:
+ client = await self._get_client()
</code_context>
<issue_to_address>
**suggestion:** Use the configurable retry count instead of the hard-coded `range(2)` in `_call_api`.
`DEFAULT_MAX_RETRIES` and the `max_retries` override aren’t honored here because `_call_api` uses `range(2)`. Please use the configured retry value (e.g., `range(self._max_retries + 1)`) so caller configuration is respected and behavior is consistent.
```suggestion
last_error: Exception | None = None
for attempt in range(self._max_retries + 1):
```
</issue_to_address>
### Comment 2
<location path="src/api/models.py" line_range="9-13" />
<code_context>
+_T = TypeVar("_T")
+
+
+def _camel_to_snake(name: str) -> str:
+ """将 camelCase 转换为 snake_case"""
+ import re
+
+ return re.sub(r"([a-z0-9])([A-Z])", r"\1_\2", name).lower()
+
+
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid importing `re` inside `_camel_to_snake` on every call.
Because `_camel_to_snake` may be called frequently via `_transform_dict`, importing `re` on each call adds avoidable overhead. Move `import re` to module scope and, if needed, precompile the regex with `re.compile` to keep this hot path efficient and clear.
</issue_to_address>
### Comment 3
<location path="src/api/models.py" line_range="39-48" />
<code_context>
+ progress: int = 0
+ max_progress: int = 0
+
+ @classmethod
+ def from_dict(cls, data: dict[str, Any]) -> "SearchCounter":
+ """从字典创建实例"""
+ data = _transform_dict(data)
+ return cls(**_filter_dataclass_fields(data, cls))
+
+
+@dataclass
+class SearchCounters:
+ """搜索计数器集合"""
+
+ pc_search: list[SearchCounter] = field(default_factory=list)
+ mobile_search: list[SearchCounter] = field(default_factory=list)
+
+ @classmethod
+ def from_dict(cls, data: dict[str, Any]) -> "SearchCounters":
+ """从字典创建实例"""
+ data = _transform_dict(data)
</code_context>
<issue_to_address>
**suggestion:** Handle non-list values for `pc_search`/`mobile_search` more defensively in `SearchCounters.from_dict`.
This currently assumes `pc_search` and `mobile_search` are always iterable lists. If the API returns `null` or a scalar, iteration will fail. Consider normalizing these keys first, e.g.:
```python
pc_raw = data.get("pc_search") or []
if not isinstance(pc_raw, list):
pc_raw = []
```
Apply similarly for `mobile_search` so `from_dict` is resilient to schema drift while keeping the same public API.
</issue_to_address>Hi @Disaster-Terminator! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并给出了一些整体性的反馈：
- 当前的重试行为被拆分在
_call_api（硬编码的range(2)，并带有各自的超时/网络重试）和get_dashboard_data（对 5xx 状态码使用self._max_retries）之间；建议将其统一起来，让所有重试逻辑都由可配置的max_retries驱动，以避免出现令人意外或重复的重试路径。 - 在
models._camel_to_snake中，每次调用函数都会在函数内部导入re；建议将该导入移动到模块级，从而避免重复导入并简化这个辅助函数。 - 目前的 HTML fallback 正则只匹配
var dashboard = ...；如果站点以后改用let/const或改变空格格式，匹配就会静默失败。建议放宽这个模式（例如允许var|let|const，并对结束符更宽松），让 fallback 更加健壮。
供 AI Agent 使用的提示词
Please address the comments from this code review:
## Overall Comments
- 当前的重试行为被拆分在 `_call_api`（硬编码的 `range(2)`，并带有各自的超时/网络重试）和 `get_dashboard_data`（对 5xx 状态码使用 `self._max_retries`）之间；建议将其统一起来，让所有重试逻辑都由可配置的 `max_retries` 驱动，以避免出现令人意外或重复的重试路径。
- 在 `models._camel_to_snake` 中，每次调用函数都会在函数内部导入 `re`；建议将该导入移动到模块级，从而避免重复导入并简化这个辅助函数。
- 目前的 HTML fallback 正则只匹配 `var dashboard = ...`；如果站点以后改用 `let`/`const` 或改变空格格式，匹配就会静默失败。建议放宽这个模式（例如允许 `var|let|const`，并对结束符更宽松），让 fallback 更加健壮。
## Individual Comments
### Comment 1
<location path="src/api/dashboard_client.py" line_range="113-114" />
<code_context>
+ "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36",
+ }
+
+ last_error: Exception | None = None
+ for attempt in range(2):
+ try:
+ client = await self._get_client()
</code_context>
<issue_to_address>
**issue (bug_risk):** _call_api hardcodes retry count instead of using configurable max_retries/retry_delay
This loop (`for attempt in range(2):`, "retrying once") bypasses `self._max_retries` and `self._retry_delay`, so the configured retry settings don’t actually control this behavior and may stack unexpectedly with `get_dashboard_data`’s retries. Please drive this loop from `self._max_retries` with `self._retry_delay` between attempts, or centralize the retry logic so it’s defined in one place.
</issue_to_address>
### Comment 2
<location path="tests/unit/test_dashboard_client.py" line_range="1" />
<code_context>
+"""Dashboard Client 单元测试"""
+
+from unittest.mock import AsyncMock, Mock
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for 401/403 auth errors where the HTML fallback also fails, to assert that `DashboardError` is raised.
Existing tests only cover cases where the HTML fallback succeeds. Please also add a test using `mock_page_no_dashboard` for 401/403 responses, asserting that `get_dashboard_data()` raises `DashboardError` when both the API and HTML fallback fail, to fully exercise the `is_auth_error` path.
Suggested implementation:
```python
from api.models import DashboardData, SearchCounters
@pytest.mark.parametrize("status_code", (401, 403))
@respx.mock
@pytest.mark.asyncio
async def test_get_dashboard_data_auth_error_with_failing_html_fallback(
status_code,
dashboard_client: DashboardClient,
mock_page_no_dashboard: str,
):
"""
When the API returns an auth error (401/403) and the HTML fallback page
also does not contain dashboard data, DashboardError should be raised.
"""
# NOTE: The exact URLs should match those used inside DashboardClient.get_dashboard_data.
# They may need to be adjusted to the real values in your test suite.
api_route = respx.get("https://example.com/api/dashboard").mock(
return_value=httpx.Response(status_code=status_code)
)
html_route = respx.get("https://example.com/dashboard").mock(
return_value=httpx.Response(status_code=200, text=mock_page_no_dashboard)
)
with pytest.raises(DashboardError):
await dashboard_client.get_dashboard_data()
# Ensure both API and HTML fallback were actually called
assert api_route.called
assert html_route.called
```
The above test assumes:
1. `dashboard_client.get_dashboard_data()` calls an API endpoint like `https://example.com/api/dashboard` and a fallback HTML endpoint like `https://example.com/dashboard`. You should update the two `respx.get(...)` URLs to exactly match the URLs used in `DashboardClient.get_dashboard_data()`, or to whatever form your existing tests use (e.g. `respx.get("https://dashboard/api/...")` or with `host="..."`).
2. A `mock_page_no_dashboard` fixture exists and returns HTML without dashboard data. If its name or type differ (e.g. it returns bytes or an `httpx.Response`), adjust the fixture parameter annotation and how it is passed into `httpx.Response` accordingly.
3. If your existing tests wrap `respx` usage differently (for example, using a shared `respx_router` fixture or different decorators), align this new test with that convention so it integrates cleanly with the rest of the test suite.
</issue_to_address>
### Comment 3
<location path="tests/unit/test_dashboard_client.py" line_range="136-145" />
<code_context>
+ assert data.user_status.available_points == 12345
+
+
+@respx.mock
+async def test_get_dashboard_data_api_error_fallback(mock_page):
+ """测试 API 错误 + HTML fallback 场景"""
+ respx.get("https://rewards.bing.com/api/getuserinfo?type=1").respond(
+ status_code=500, text="Internal Server Error"
+ )
+
+ client = DashboardClient(mock_page)
+ data = await client.get_dashboard_data()
+ assert isinstance(data, DashboardData)
+ assert data.user_status.available_points == 12345
+
+
+@respx.mock
+async def test_get_dashboard_data_unauthorized_fallback(mock_page):
+ """测试 401 错误 + HTML fallback 场景"""
+ respx.get("https://rewards.bing.com/api/getuserinfo?type=1").respond(
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for `get_current_points` and `get_search_counters` when they rely on HTML fallback rather than a successful API response.
Right now, only the `get_dashboard_data_*` tests cover HTML fallback; `get_current_points` and `get_search_counters` are only tested for the 200 + valid JSON path. Please add tests where the API returns 5xx or times out and the HTML contains a valid `var dashboard = {...}` (like `mock_page`), asserting that `get_current_points` returns the correct points and `get_search_counters` builds a populated `SearchCounters` from the HTML data.
Suggested implementation:
```python
@respx.mock
async def test_get_current_points_api_error_html_fallback(mock_page):
"""API 5xx 时应从 HTML fallback 中获取当前积分"""
respx.get("https://rewards.bing.com/api/getuserinfo?type=1").respond(
status_code=500, text="Internal Server Error"
)
client = DashboardClient(mock_page)
points = await client.get_current_points()
# HTML 中的 dashboard 数据应与正常场景一致
assert points == 12345
@respx.mock
async def test_get_search_counters_timeout_html_fallback(mock_page):
"""API 超时时应从 HTML fallback 中构建搜索计数器"""
respx.get("https://rewards.bing.com/api/getuserinfo?type=1").mock(
side_effect=httpx.TimeoutException("Request timed out")
)
client = DashboardClient(mock_page)
counters = await client.get_search_counters()
# 确认从 HTML 中成功构建了 SearchCounters，而不是返回空数据
# 这里假设 HTML 中提供了 PC 和 Mobile 的搜索计数
assert counters is not None
assert getattr(counters, "pc", None) is not None
assert getattr(counters, "mobile", None) is not None
@respx.mock
```
1. 确保本测试文件顶部已经 `import httpx`，否则需要补充：
`import httpx`。
2. 如果在现有测试中已经对 `SearchCounters` 的字段有更精确的断言（例如 `pc.remaining == 10` 等），建议在 `test_get_search_counters_timeout_html_fallback` 中使用相同的期望值来替换当前的通用属性检查，以保证与 HTML mock 数据严格对齐。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据这些反馈改进后续的 Review。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- The retry behavior is split between
_call_api(hard-codedrange(2)with its own timeout/network retries) andget_dashboard_data(usingself._max_retriesfor 5xx status codes); consider unifying this so that all retry logic is driven by the configurablemax_retriesto avoid surprising or duplicated retry paths. - In
models._camel_to_snakeyou importreinside the function on every call; moving this import to the module level will avoid repeated imports and simplify the helper. - The HTML fallback regex currently only matches
var dashboard = ...; if the site ever switches tolet/constor changes spacing, this will silently fail, so consider broadening the pattern (e.g. allowingvar|let|constand more flexible terminators) to make the fallback more robust.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The retry behavior is split between `_call_api` (hard-coded `range(2)` with its own timeout/network retries) and `get_dashboard_data` (using `self._max_retries` for 5xx status codes); consider unifying this so that all retry logic is driven by the configurable `max_retries` to avoid surprising or duplicated retry paths.
- In `models._camel_to_snake` you import `re` inside the function on every call; moving this import to the module level will avoid repeated imports and simplify the helper.
- The HTML fallback regex currently only matches `var dashboard = ...`; if the site ever switches to `let`/`const` or changes spacing, this will silently fail, so consider broadening the pattern (e.g. allowing `var|let|const` and more flexible terminators) to make the fallback more robust.
## Individual Comments
### Comment 1
<location path="src/api/dashboard_client.py" line_range="113-114" />
<code_context>
+ "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36",
+ }
+
+ last_error: Exception | None = None
+ for attempt in range(2):
+ try:
+ client = await self._get_client()
</code_context>
<issue_to_address>
**issue (bug_risk):** _call_api hardcodes retry count instead of using configurable max_retries/retry_delay
This loop (`for attempt in range(2):`, "retrying once") bypasses `self._max_retries` and `self._retry_delay`, so the configured retry settings don’t actually control this behavior and may stack unexpectedly with `get_dashboard_data`’s retries. Please drive this loop from `self._max_retries` with `self._retry_delay` between attempts, or centralize the retry logic so it’s defined in one place.
</issue_to_address>
### Comment 2
<location path="tests/unit/test_dashboard_client.py" line_range="1" />
<code_context>
+"""Dashboard Client 单元测试"""
+
+from unittest.mock import AsyncMock, Mock
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for 401/403 auth errors where the HTML fallback also fails, to assert that `DashboardError` is raised.
Existing tests only cover cases where the HTML fallback succeeds. Please also add a test using `mock_page_no_dashboard` for 401/403 responses, asserting that `get_dashboard_data()` raises `DashboardError` when both the API and HTML fallback fail, to fully exercise the `is_auth_error` path.
Suggested implementation:
```python
from api.models import DashboardData, SearchCounters
@pytest.mark.parametrize("status_code", (401, 403))
@respx.mock
@pytest.mark.asyncio
async def test_get_dashboard_data_auth_error_with_failing_html_fallback(
status_code,
dashboard_client: DashboardClient,
mock_page_no_dashboard: str,
):
"""
When the API returns an auth error (401/403) and the HTML fallback page
also does not contain dashboard data, DashboardError should be raised.
"""
# NOTE: The exact URLs should match those used inside DashboardClient.get_dashboard_data.
# They may need to be adjusted to the real values in your test suite.
api_route = respx.get("https://example.com/api/dashboard").mock(
return_value=httpx.Response(status_code=status_code)
)
html_route = respx.get("https://example.com/dashboard").mock(
return_value=httpx.Response(status_code=200, text=mock_page_no_dashboard)
)
with pytest.raises(DashboardError):
await dashboard_client.get_dashboard_data()
# Ensure both API and HTML fallback were actually called
assert api_route.called
assert html_route.called
```
The above test assumes:
1. `dashboard_client.get_dashboard_data()` calls an API endpoint like `https://example.com/api/dashboard` and a fallback HTML endpoint like `https://example.com/dashboard`. You should update the two `respx.get(...)` URLs to exactly match the URLs used in `DashboardClient.get_dashboard_data()`, or to whatever form your existing tests use (e.g. `respx.get("https://dashboard/api/...")` or with `host="..."`).
2. A `mock_page_no_dashboard` fixture exists and returns HTML without dashboard data. If its name or type differ (e.g. it returns bytes or an `httpx.Response`), adjust the fixture parameter annotation and how it is passed into `httpx.Response` accordingly.
3. If your existing tests wrap `respx` usage differently (for example, using a shared `respx_router` fixture or different decorators), align this new test with that convention so it integrates cleanly with the rest of the test suite.
</issue_to_address>
### Comment 3
<location path="tests/unit/test_dashboard_client.py" line_range="136-145" />
<code_context>
+ assert data.user_status.available_points == 12345
+
+
+@respx.mock
+async def test_get_dashboard_data_api_error_fallback(mock_page):
+ """测试 API 错误 + HTML fallback 场景"""
+ respx.get("https://rewards.bing.com/api/getuserinfo?type=1").respond(
+ status_code=500, text="Internal Server Error"
+ )
+
+ client = DashboardClient(mock_page)
+ data = await client.get_dashboard_data()
+ assert isinstance(data, DashboardData)
+ assert data.user_status.available_points == 12345
+
+
+@respx.mock
+async def test_get_dashboard_data_unauthorized_fallback(mock_page):
+ """测试 401 错误 + HTML fallback 场景"""
+ respx.get("https://rewards.bing.com/api/getuserinfo?type=1").respond(
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for `get_current_points` and `get_search_counters` when they rely on HTML fallback rather than a successful API response.
Right now, only the `get_dashboard_data_*` tests cover HTML fallback; `get_current_points` and `get_search_counters` are only tested for the 200 + valid JSON path. Please add tests where the API returns 5xx or times out and the HTML contains a valid `var dashboard = {...}` (like `mock_page`), asserting that `get_current_points` returns the correct points and `get_search_counters` builds a populated `SearchCounters` from the HTML data.
Suggested implementation:
```python
@respx.mock
async def test_get_current_points_api_error_html_fallback(mock_page):
"""API 5xx 时应从 HTML fallback 中获取当前积分"""
respx.get("https://rewards.bing.com/api/getuserinfo?type=1").respond(
status_code=500, text="Internal Server Error"
)
client = DashboardClient(mock_page)
points = await client.get_current_points()
# HTML 中的 dashboard 数据应与正常场景一致
assert points == 12345
@respx.mock
async def test_get_search_counters_timeout_html_fallback(mock_page):
"""API 超时时应从 HTML fallback 中构建搜索计数器"""
respx.get("https://rewards.bing.com/api/getuserinfo?type=1").mock(
side_effect=httpx.TimeoutException("Request timed out")
)
client = DashboardClient(mock_page)
counters = await client.get_search_counters()
# 确认从 HTML 中成功构建了 SearchCounters，而不是返回空数据
# 这里假设 HTML 中提供了 PC 和 Mobile 的搜索计数
assert counters is not None
assert getattr(counters, "pc", None) is not None
assert getattr(counters, "mobile", None) is not None
@respx.mock
```
1. 确保本测试文件顶部已经 `import httpx`，否则需要补充：
`import httpx`。
2. 如果在现有测试中已经对 `SearchCounters` 的字段有更精确的断言（例如 `pc.remaining == 10` 等），建议在 `test_get_search_counters_timeout_html_fallback` 中使用相同的期望值来替换当前的通用属性检查，以保证与 HTML mock 数据严格对齐。
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Disaster-Terminator
force-pushed
the
feature/dashboard-api
branch
5 times, most recently
from
5a609df to
43a463b
Compare
- 移除宽松的 bing.com 域名匹配，仅允许 API 域名及其子域名 - 更新测试用例以匹配严格的域名过滤逻辑 - 防止跨域 Cookie 泄露到不相关的子域名
Disaster-Terminator
force-pushed
the
feature/dashboard-api
branch
from
43a463b to
156d234
Compare
Owner
Author
|
/agentic_review |
Owner
Author
|
@sourcery-ai review |
|
Persistent review updated to latest commit 156d234 |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性反馈：
- DashboardClient 目前在
close()中将_client设为None，但_call_api假定它始终是一个存活的 client；建议对 httpx client 进行惰性初始化（或在为None时重新创建），这样在close()之后或通过上下文管理器多次使用该实例时，都能安全复用。 _call_api（处理超时/网络错误）和get_dashboard_data（处理 5xx 错误）中存在嵌套的重试循环，这会让实际的重试行为更难以推理；建议将重试策略集中到一个地方，从而使控制流和时序特性更清晰。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- DashboardClient 目前在 `close()` 中将 `_client` 设为 `None`，但 `_call_api` 假定它始终是一个存活的 client；建议对 httpx client 进行惰性初始化（或在为 `None` 时重新创建），这样在 `close()` 之后或通过上下文管理器多次使用该实例时，都能安全复用。
- `_call_api`（处理超时/网络错误）和 `get_dashboard_data`（处理 5xx 错误）中存在嵌套的重试循环，这会让实际的重试行为更难以推理；建议将重试策略集中到一个地方，从而使控制流和时序特性更清晰。
## Individual Comments
### Comment 1
<location path="src/api/dashboard_client.py" line_range="136" />
<code_context>
+
+ for attempt in range(self._max_retries + 1):
+ try:
+ response = await self._client.get(self._api_url, headers=headers)
+ response.raise_for_status()
+ data = response.json()
</code_context>
<issue_to_address>
**issue:** `_client` 可以被 `close()` 设为 `None`，但 `_call_api` 假定它始终是一个 `AsyncClient`。
如果在调用 `close()` 之后再调用 `get_dashboard_data()` 等方法，目前会在 `self._client.get` 处因 `AttributeError` 而失败。建议要么处理 `self._client is None` 的情况（例如，重新创建 client），要么通过在 `close()` 之后使用这些方法时抛出明确的领域错误来强制执行更严格的生命周期管理。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据反馈改进 Review 质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- DashboardClient currently sets
_clienttoNoneinclose()but_call_apiassumes it is always a live client; consider lazy-initializing the httpx client (or recreating it ifNone) so that the instance can be safely reused after being closed or when used via the context manager multiple times. - There are nested retry loops in
_call_api(for timeout/network errors) andget_dashboard_data(for 5xx errors), which makes the effective retry behaviour harder to reason about; consider centralising the retry policy in one place to keep the control flow and timing characteristics clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- DashboardClient currently sets `_client` to `None` in `close()` but `_call_api` assumes it is always a live client; consider lazy-initializing the httpx client (or recreating it if `None`) so that the instance can be safely reused after being closed or when used via the context manager multiple times.
- There are nested retry loops in `_call_api` (for timeout/network errors) and `get_dashboard_data` (for 5xx errors), which makes the effective retry behaviour harder to reason about; consider centralising the retry policy in one place to keep the control flow and timing characteristics clearer.
## Individual Comments
### Comment 1
<location path="src/api/dashboard_client.py" line_range="136" />
<code_context>
+
+ for attempt in range(self._max_retries + 1):
+ try:
+ response = await self._client.get(self._api_url, headers=headers)
+ response.raise_for_status()
+ data = response.json()
</code_context>
<issue_to_address>
**issue:** `_client` can be set to `None` by `close()`, but `_call_api` assumes it is always an `AsyncClient`.
If methods like `get_dashboard_data()` are called after `close()`, this will currently fail with an `AttributeError` on `self._client.get`. Consider either handling `self._client is None` (e.g., recreating the client) or enforcing a stricter lifecycle by raising a clear domain error when methods are used after `close()`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- 添加 _client None 检查，防止 close() 后调用 - from_dict 增加列表元素类型校验 - 嵌套对象字段增加类型校验 - Cookie 选择改用 Playwright URL 作用域 - 测试文件添加 sys.path 注入 - pyproject.toml 添加 respx 到 test 依赖 - points_text 添加 None 检查
Disaster-Terminator
force-pushed
the
feature/dashboard-api
branch
from
4653561 to
bd2be06
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
实现 Dashboard API 客户端集成，通过 API 优先获取积分数据，提高数据获取的可靠性和性能。
变更内容
新增文件
src/api/__init__.py- API 模块入口src/api/models.py- Dashboard 数据模型 (dataclass)src/api/dashboard_client.py- Dashboard API 客户端tests/unit/test_dashboard_client.py- 单元测试 (13 个测试用例)修改文件
src/account/points_detector.py- 集成 DashboardClient，优先使用 APIpyproject.toml- 添加 respx 测试依赖核心功能
DashboardClient
https://rewards.bing.com/api/getuserinfo?type=1异常处理
httpx.RequestError,httpx.TimeoutException)Exception捕获，防止掩盖系统错误测试覆盖
验证结果
Summary by Sourcery
引入可复用、带类型的 Dashboard API 客户端和数据模型，并将其集成到积分检测中，以优先使用基于 API 的获取方式，并具备稳健的回退机制。
New Features:
DashboardClient，用于消费 Microsoft Rewards Dashboard API，包括对积分和搜索计数器的支持。api包对与 dashboard 相关的客户端和数据模型进行暴露，以便在整个项目中复用。Enhancements:
Build:
httpx作为运行时依赖，以及respx作为开发依赖，用于在测试中对 HTTP 客户端进行模拟。api和constants包识别为一方模块（first-party modules）。Documentation:
CURRENT_TASK文档，描述 Dashboard API 集成的目标、设计和验收标准。Tests:
DashboardClient添加全面的单元测试，覆盖 API 成功场景、各种回退路径、重试行为、超时、网络错误、Cookie 过滤以及数据模型解析的边界情况。Original summary in English
Summary by Sourcery
Introduce a reusable, typed Dashboard API client and data models, and integrate it into points detection to prefer API-based retrieval with robust fallbacks.
New Features:
Enhancements:
Build:
Documentation:
Tests:
Original summary in English
Summary by Sourcery
引入可复用、带类型的 Dashboard API 客户端和数据模型，并将其集成到积分检测中，以优先使用基于 API 的获取方式，并具备稳健的回退机制。
New Features:
DashboardClient，用于消费 Microsoft Rewards Dashboard API，包括对积分和搜索计数器的支持。api包对与 dashboard 相关的客户端和数据模型进行暴露，以便在整个项目中复用。Enhancements:
Build:
httpx作为运行时依赖，以及respx作为开发依赖，用于在测试中对 HTTP 客户端进行模拟。api和constants包识别为一方模块（first-party modules）。Documentation:
CURRENT_TASK文档，描述 Dashboard API 集成的目标、设计和验收标准。Tests:
DashboardClient添加全面的单元测试，覆盖 API 成功场景、各种回退路径、重试行为、超时、网络错误、Cookie 过滤以及数据模型解析的边界情况。Original summary in English
Summary by Sourcery
Introduce a reusable, typed Dashboard API client and data models, and integrate it into points detection to prefer API-based retrieval with robust fallbacks.
New Features:
Enhancements:
Build:
Documentation:
Tests:
新功能：
DashboardClient以及相关数据模型，用于消费 Bing Rewards Dashboard API，包括对积分、搜索计数器、促销活动以及打卡卡片（punch cards）的支持。api包对外暴露 Dashboard API 客户端及模型，以便在整个项目中复用。增强优化：
构建：
respx添加为开发依赖，用于在测试中对 HTTP 客户端进行模拟（mock）。文档：
CURRENT_TASK文档，描述 Dashboard API 集成的目标、设计和验收标准。测试：
DashboardClient添加全面的单元测试，覆盖成功的 API 调用、各种回退场景、重试逻辑、超时以及网络错误等情况。Original summary in English
Summary by Sourcery
引入可复用、带类型的 Dashboard API 客户端和数据模型，并将其集成到积分检测中，以优先使用基于 API 的获取方式，并具备稳健的回退机制。
New Features:
DashboardClient，用于消费 Microsoft Rewards Dashboard API，包括对积分和搜索计数器的支持。api包对与 dashboard 相关的客户端和数据模型进行暴露，以便在整个项目中复用。Enhancements:
Build:
httpx作为运行时依赖，以及respx作为开发依赖，用于在测试中对 HTTP 客户端进行模拟。api和constants包识别为一方模块（first-party modules）。Documentation:
CURRENT_TASK文档，描述 Dashboard API 集成的目标、设计和验收标准。Tests:
DashboardClient添加全面的单元测试，覆盖 API 成功场景、各种回退路径、重试行为、超时、网络错误、Cookie 过滤以及数据模型解析的边界情况。Original summary in English
Summary by Sourcery
Introduce a reusable, typed Dashboard API client and data models, and integrate it into points detection to prefer API-based retrieval with robust fallbacks.
New Features:
Enhancements:
Build:
Documentation:
Tests:
Original summary in English
Summary by Sourcery
引入可复用、带类型的 Dashboard API 客户端和数据模型，并将其集成到积分检测中，以优先使用基于 API 的获取方式，并具备稳健的回退机制。
New Features:
DashboardClient，用于消费 Microsoft Rewards Dashboard API，包括对积分和搜索计数器的支持。api包对与 dashboard 相关的客户端和数据模型进行暴露，以便在整个项目中复用。Enhancements:
Build:
httpx作为运行时依赖，以及respx作为开发依赖，用于在测试中对 HTTP 客户端进行模拟。api和constants包识别为一方模块（first-party modules）。Documentation:
CURRENT_TASK文档，描述 Dashboard API 集成的目标、设计和验收标准。Tests:
DashboardClient添加全面的单元测试，覆盖 API 成功场景、各种回退路径、重试行为、超时、网络错误、Cookie 过滤以及数据模型解析的边界情况。Original summary in English
Summary by Sourcery
Introduce a reusable, typed Dashboard API client and data models, and integrate it into points detection to prefer API-based retrieval with robust fallbacks.
New Features:
Enhancements:
Build:
Documentation:
Tests: