✨ feat(timing): 实现提醒触发查询 - #178
Conversation
为提醒规则管理提供原子批量写入,避免逐条保存造成部分更新。 服务校验 weak/strong snooze、日程归属和准点 strong 唯一性;内存 fake 与主机测试覆盖成功、冲突和写入失败路径。 Host、架构、Profile、Python 检查及 Node 24 Gateway CI 已通过;本机缺少 GNU GCC,gcov 覆盖率由 CI 确认。 Closes 1024XEngineer#141
服务层基于旧快照校验后写入时,多个并发请求可能绕过准点强提醒唯一性。 Store Port 现在要求在同一原子写入边界复核该不变量;内存 fake 和 Store 合同测试覆盖第二个 writer 被拒绝的场景。 ./scripts/run_checks.sh 已通过。 Refs 1024XEngineer#141
实现左闭右开时间范围内的用户可见 occurrence 查询,支持日程和状态过滤、排序与分页。通过 Store Port 查询任务和已物化实例,并在不引入 RFC 5545/IANA tzdb 的前提下展开基础周期规则,使用 modified/completed/skipped 例外覆盖基础 occurrence。 RED:ListCalendarView 合法范围测试在原 kUnavailable stub 上失败。GREEN:补充最小查询 Port 和内存 fake 后通过一次性、周期展开、例外覆盖、过滤分页及非法输入测试。REFACTOR:将查询实现独立到日历翻译单元。 Refs 1024XEngineer#143
…ar-view # Conflicts: # tests/host/timing_task_service_test.cc
补充周、月、年周期展开,Store 查询失败、分页参数和过滤分支的公开 Service 测试,满足 C++ patch 覆盖率门禁。 Refs 1024XEngineer#143
周期规则使用 UTC 民用日期展开;对其他时区返回明确的不可用错误,避免静默产生错误的星期、月日和年月日匹配。补充 Host 失败路径测试。\n\nRefs 1024XEngineer#143
按任务、实例、日程、类型、状态和实际触发时间范围查询已物化提醒触发,支持稳定排序和分页。Store Port 增加只读触发和实例查询,未引入触发生成、消息投递或生产数据库实现。 RED:公开 Service 查询测试在 kUnavailable 占位实现上失败。GREEN:补齐最小查询 Port、内存 fake 和组合过滤实现。REFACTOR:将查询隔离为独立翻译单元并明确契约时间语义。 已通过完整提交前门禁;真实 SQLite Adapter 仍属后续范围。 Refs 1024XEngineer#144
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Reference review: Codecov report结论:不需要为该报告追加修复。
因此该 Codecov 评论提供的是可见性信息,不构成超出 #144 验收的补测理由;不新增代码或提交。 |
|
@fennoai 你是资深后端/全栈架构师 + 嵌入式硬件工程师复合型专家,执行严格、客观、不留情面的代码仓库Review,请遵循下面所有评审规则,逐条输出审查结果,禁止敷衍、禁止只说空话、禁止笼统概括。 评审维度
输出要求
现在开始对提供的仓库代码/PR变更、硬件资料、结构模型进行评审 |
| return Result<CalendarView>::Failure(ErrorCode::kUnavailable, "周期日历展开暂仅支持 UTC 时区"); | ||
| } | ||
|
|
||
| const auto instances = store_.ListInstances(task.id); |
There was a problem hiding this comment.
This fetch runs for every active task before you know whether the task is recurring. For one-off tasks, ListInstances is unused, so a transient instance-store failure now aborts the whole calendar read even though the task could still be rendered from task.start_at alone.
| .actual_trigger_at = instance.actual_trigger_at, | ||
| .status = instance.status, | ||
| .is_recurring = task.recurrence.frequency != RecurrenceFrequency::kNone, | ||
| .is_exception = true, |
There was a problem hiding this comment.
Every persisted instance is marked as is_exception=true, even when it has no override fields. That makes normal materialized occurrences indistinguishable from real exceptions in the calendar response.
严格 Review 结果结论:不建议直接合并到生产路径。Host 测试能通过,但当前 Store Port 和 Service 实现把查询下推、分页、权限/可见性、边界值语义都留给未来适配器或调用方,嵌入式长期运行风险明显。本次未发现 PR 中包含硬件资料、结构模型、原理图或驱动改动,因此引脚、电平、电流、电机限位等硬件项无法实物级核验。 问题清单
整体仓库风险总结当前实现满足 Host 内存 fake 的功能验证,但生产级查询边界没有建立:过滤、排序、分页、可见性和索引没有进入 Store 合同,应用层全量扫描会在长期运行设备上累积成 RAM、Flash IO、功耗和看门狗风险。安全方面,列表接口缺少调用方/租户/投影控制, 优先级整改清单
长期架构与软硬件协同优化方案把 TimingTask 查询建成“应用服务编排 + Store 索引查询 + DTO 投影”的三层合同:Service 负责参数和资源关系校验,Store 负责索引化过滤分页,Adapter 负责 SQLite/NVS/Flash 的事务和索引。提醒通道不要继续使用裸字符串,改成枚举或注册表,并在硬件/语音/IM 适配层集中做 allowlist、超时、重试、故障状态上报。硬件资料必须和固件版本绑定:引脚、外设电平、通信协议校验和、断线重连、异常状态码都要进入仓库并纳入测试或仿真,否则后续上机风险不可控。 验证
|
参考审查:fennoai review 的范围与验收核对结论:#144 不需要改代码,因此不会向本 PR 增加提交。 范围依据
继承日历代码的行级意见
编号意见逐项判断1-4、6、19 - 本切片不采纳。 Store 端过滤/分页/排序、索引和大历史数据性能属于生产 Adapter/索引设计;#144 与 #137 已明确排除。当前最小 Port/fake 正是允许的纵向切片。 两条日历意见应在 #171/#143 基于其最新依赖头继续评估。#144 的实现保持在已接受的查询契约内,不修改继承的日历/规则行为。 |
结论
完成 #144 的
TimingTaskService::ListReminderTriggers:支持任务、实例、日程、类型、状态和实际触发时间的组合查询,并返回稳定排序的分页结果。本 PR 依赖开放 PR #171(#143)。由于 GitHub 不允许把个人 fork 的 #171 分支作为本 PR 的 base,当前 diff 会包含其依赖提交;请先审 #171,再审本 PR 最后的提交
4cd7fe9。背景
父 Issue #137 要求逐个完成 TimingTask 的公开 Service,同时维持 Domain、Application 与 Port/Adapter 边界。本切片只读取已物化的提醒触发。
改动范围
ListTriggers与FindInstance最小 Store Port,并提供内存 fake。total和has_more。接口与依赖影响
TimingTaskStorePort增加两个只读方法;后续真实 Adapter 需实现相同行为合同。未新增跨组件依赖,未变更 Profile、协议或持久化格式。TDD 与验证
kUnavailable占位实现上失败。./scripts/run_pre_submit_checks.sh已通过:C++ Host 22/22、架构/Profile/Python 检查、IM Gateway 126/126。已知风险、兼容与回退
生产 SQLite/NVS Adapter 尚未实现,当前证据仅覆盖 Host 内存 fake。该 PR 依赖 #171 先合入;其合入后应以最新
main重新基线。回退方式为回退本 PR 的单个提交4cd7fe9,即可恢复原有kUnavailable占位实现。Refs #144
Refs #137
Refs #171