Skip to content

[refactor] PR3-按页面 URL 区分历史进度#2275

Draft
liangyuR wants to merge 2 commits into
Predidit:mainfrom
liangyuR:refactor/pr3-history-pageurl
Draft

[refactor] PR3-按页面 URL 区分历史进度#2275
liangyuR wants to merge 2 commits into
Predidit:mainfrom
liangyuR:refactor/pr3-history-pageurl

Conversation

@liangyuR

Copy link
Copy Markdown
Contributor

摘要

本 PR 为统一集数身份的 PR3:历史进度记录新增单条 Progress.episodePageUrl,让播放历史在同一集数编号但不同页面 URL 的场景下可以按 URL 精确恢复。

主要变更

  • Progress 持久化 episodePageUrl,并保持旧数据缺字段时默认为空字符串。
  • 历史仓储查询、更新和清除进度时优先按 URL 匹配;URL 不存在或旧数据未写入时回退到原有集数匹配,并在安全路径懒回填 URL。
  • 在线播放恢复时优先通过 episodePageUrl 在当前分线路列表中重新定位,源列表重排后仍能恢复到正确条目。
  • 历史同步的 progress payload 复用 episodePageUrl 表示单条进度 URL,watch-state 仍表示最后观看 URL;旧 payload 缺字段继续兼容。
  • 补充仓储、同步和播放定位测试,覆盖 URL 优先、旧数据回填、synthetic bucket 以及同步 JSON 兼容场景。

兼容性

  • 不做全量 Hive 扫描迁移;旧历史数据通过查询、更新、同步路径懒回填。
  • 空 URL 调用仍保留旧的集数回退行为,不覆盖已有不同 URL 的进度记录。

验证

  • flutter test test/history_repository_test.dart test/history_sync_test.dart test/episode_ref_test.dart test/episode_url_test.dart
  • flutter analyze --no-fatal-infos

- 为单条 Progress 记录 episodePageUrl 并优先按 URL 匹配

- 兼容旧集数进度并在查询、更新、同步路径懒回填 URL

- 增加历史仓储、同步和播放恢复定位测试
@liangyuR liangyuR changed the title 按页面 URL 区分历史进度 [refactor] 按页面 URL 区分历史进度 Jun 26, 2026
@liangyuR liangyuR changed the title [refactor] 按页面 URL 区分历史进度 [refactor] PR3-按页面 URL 区分历史进度 Jun 26, 2026
@liangyuR liangyuR marked this pull request as ready for review June 26, 2026 15:57
@kilo-code-bot

kilo-code-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review of the migrateProgressPageUrls addition (since f350b15). The two-phase plan/apply migration is internally consistent: it keys planned rewrites by bucket while resolving the current URL from the stored Progress.road/episode (correctly handling synthetic buckets), resolves target-URL collisions by keeping the newer updatedAtMs and removing the loser, and synchronizes the top-level history.episodePageUrl so getLastWatchingProgress still matches the surviving bucket. Empty/unresolvable URLs and no-op cases short-circuit before any write, and the whole routine is wrapped in try/catch with logging. The synchronous invocation in _initOnlineMode mutates the in-memory History instance before lastWatching reads it, so the post-migration URL is observed immediately despite the fire-and-forget box write. The added repository tests exercise in-place rewrite + bucket reuse, top-level URL migration, synthetic buckets, duplicate collapse, and empty-resolver paths.

Files Reviewed (5 files)
  • lib/pages/history/history_controller.dart
  • lib/pages/video/video_controller.dart
  • lib/pages/video/video_page.dart
  • lib/repositories/history_repository.dart
  • test/history_repository_test.dart
Previous Review Summary (commit f350b15)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit f350b15)

Status: No Issues Found | Recommendation: Merge

Reviewed the PR3 change that adds per-Progress.episodePageUrl tracking. The URL-priority matching (_HistoryEpisodeMatcher.find / bucketForNewProgress), lazy backfill, synthetic-bucket allocation, sync merge logic, and the page-URL based resume selection (findEpisodeSelectionByPageUrl + video_page.dart) are internally consistent and well covered by the added repository/sync/locator tests. Legacy-data and empty-URL fallback paths behave as documented.

Files Reviewed (11 files)
  • lib/modules/history/history_module.dart
  • lib/modules/history/history_module.g.dart (generated)
  • lib/modules/history/history_sync.dart
  • lib/pages/history/history_controller.dart
  • lib/pages/video/video_controller.dart
  • lib/pages/video/video_page.dart
  • lib/repositories/history_repository.dart
  • lib/services/sync/history_sync_service.dart
  • test/episode_ref_test.dart
  • test/history_repository_test.dart
  • test/history_sync_test.dart

Reviewed by claude-4.8-opus-20260528 · Input: 4.7K · Output: 9.5K · Cached: 472.9K

@Predidit

Copy link
Copy Markdown
Owner

如果我的理解没有错误,这里的设计可能有一个小问题

假设我们更新了某条规则的 baseURL ,那么在升级之后,历史记录里作为身份凭据的 pageURL 将不再匹配,虽然我们通过 fallback 路径可以正确播放,但是我们将记录新的历史记录

这意味着规则没升级一次,都会产生同一规则的重复历史记录,这符合预期吗

以及一个代码清洁问题

_HistoryEpisodeMatcher 被几乎完全相同地实现了两次

@liangyuR liangyuR marked this pull request as draft June 27, 2026 04:57
@liangyuR

Copy link
Copy Markdown
Contributor Author

如果我的理解没有错误,这里的设计可能有一个小问题

假设我们更新了某条规则的 baseURL ,那么在升级之后,历史记录里作为身份凭据的 pageURL 将不再匹配,虽然我们通过 fallback 路径可以正确播放,但是我们将记录新的历史记录

这意味着规则没升级一次,都会产生同一规则的重复历史记录,这符合预期吗


我认为规则 baseURL 升级不应污染观看历史,目前是不符合预期的,我还没有比较合适的思路来解决这个问题。

@liangyuR

Copy link
Copy Markdown
Contributor Author

两种思路:

  1. 当 fallback 命中时,迁移旧的 pageURL,改动比较小,但我认为不太优雅。
  2. 新增一种 “路径身份Key” 不含 host,这样能从根源避免 baseURL 的分裂,但是改动比较大

我想知道你觉得哪种合适?

@liangyuR liangyuR closed this Jun 27, 2026
@liangyuR liangyuR deleted the refactor/pr3-history-pageurl branch June 27, 2026 05:45
@liangyuR liangyuR restored the refactor/pr3-history-pageurl branch June 27, 2026 05:45
@liangyuR liangyuR reopened this Jun 27, 2026
@Predidit

Copy link
Copy Markdown
Owner

我觉得第一种方案好一些

因为第二种所谓从根本上解决问题的方案页不靠谱,路径发生变化所以更新规则也是常见的事情

When a rule's baseURL changes, history progress entries keep the old
normalized pageURL, so playback falls back to episode/road matching but
saving allocates a new bucket, creating a duplicate progress per upgrade.

Add migrateProgressPageUrls to rewrite stale URLs in place (resolved by
the Progress value's road/episode against the current roadList) when the
online video page opens, collapsing any duplicate buckets. Subsequent
writes then reuse the existing entry instead of duplicating it.

Co-authored-by: Cursor <cursoragent@cursor.com>
@liangyuR liangyuR marked this pull request as ready for review June 27, 2026 09:59
@Predidit

Copy link
Copy Markdown
Owner

还是有些奇怪 代码在尝试用旧 URL 查找剧集之前,就先用旧的列表位置覆盖了旧 URL

简单说,如果是剧集全部更新完之后的剧集重排 (原来最新集在前,现在第一集在前),最新变更前会进行类似 列表重排 的处理,现在统一理解为 baseURL 已经变化

我总觉得所有问题都来自这一系列设计的前提,也就是以 URL 为判据并非不合理,但是要考虑其变化,不能以其不变为前提进行设计

我们可以先回滚最新变更,前一个变更位置其实不会出现重复历史记录卡片,虽然其内部有记录重复,但是看上去不是那么重要

@liangyuR

Copy link
Copy Markdown
Contributor Author

我要思考一天 - -

暂时不打算改了,一定有更合适的解决方案。

@liangyuR liangyuR marked this pull request as draft June 27, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants