perf: enhance fallback cache mechanism#223
Merged
Soulter merged 2 commits intoAstrBotDevs:mainfrom Dec 9, 2025
Merged
Conversation
There was a problem hiding this comment.
你好,我已经审查了你的修改,这里有一些反馈:
- 新的 jq 表达式在没有先检查
.value.repo是否存在的情况下,直接索引$repos[$plugin.value.repo],这可能会在插件缺少repo字段时触发Cannot index object with null错误;建议和之前的版本一样,对repo的存在性做显式检查后再访问。 map(...)代码块中的 jq 逻辑现在相当复杂,包含很多中间变量和嵌套条件;建议将部分回退/合并逻辑拆分到命名的辅助过滤器或清晰分隔的代码块中,以便未来维护和调试。
面向 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- 新的 jq 表达式在没有先检查 `.value.repo` 是否存在的情况下,直接索引 `$repos[$plugin.value.repo]`,这可能会在插件缺少 `repo` 字段时触发 `Cannot index object with null` 错误;建议和之前的版本一样,对 `repo` 的存在性做显式检查后再访问。
- `map(...)` 代码块中的 jq 逻辑现在相当复杂,包含很多中间变量和嵌套条件;建议将部分回退/合并逻辑拆分到命名的辅助过滤器或清晰分隔的代码块中,以便未来维护和调试。
## Individual Comments
### Comment 1
<location> `scripts/transform_plugin_data/transform_plugin_data/run.sh:3-7` </location>
<code_context>
-map(select(
- if .value.repo and ($repo_info[0][.value.repo]) then
- ($repo_info[0][.value.repo].status != "deleted")
+existing_cache_file="existing_cache.json"
+cleanup_existing_cache="false"
+
+# 如果没有历史缓存,为jq提供一个空对象以便统一逻辑
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 临时缓存文件目前只在“正常结束”的路径上被清理,这在发生错误时可能会遗留多余文件。
`existing_cache_file` 是通过 `mktemp` 创建的,但只会在脚本执行到最终清理步骤时才被删除。如果 `jq` 或其他命令中途失败,这个临时文件会被留下。可以考虑添加 `set -euo pipefail` 和 `trap`,以确保在退出或出错时总是清理 `existing_cache_file`。
```suggestion
set -euo pipefail
# 始终在脚本退出时尝试清理临时缓存文件,避免错误情况下遗留
cleanup_existing_cache="false"
existing_cache_file="existing_cache.json"
cleanup() {
# 仅在当前执行中创建的临时缓存文件才会被清理
if [ "${cleanup_existing_cache}" = "true" ] && [ -n "${existing_cache_file:-}" ] && [ -f "${existing_cache_file}" ]; then
rm -f "${existing_cache_file}"
fi
}
trap cleanup EXIT INT TERM
echo "开始转换插件数据格式..."
# 使用jq转换数据格式,增加容错处理,并过滤掉404的仓库
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的审查质量。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- The new jq expression directly indexes
$repos[$plugin.value.repo]without first checking that.value.repoexists, which can raiseCannot index object with nullfor plugins without arepofield; consider guarding this access with an explicit repo-presence check as in the previous version. - The jq logic in the
map(...)block has become fairly complex with many intermediate variables and nested conditionals; consider breaking out some of the fallback/merging logic into named helper filters or clearly separated blocks to make future maintenance and debugging easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new jq expression directly indexes `$repos[$plugin.value.repo]` without first checking that `.value.repo` exists, which can raise `Cannot index object with null` for plugins without a `repo` field; consider guarding this access with an explicit repo-presence check as in the previous version.
- The jq logic in the `map(...)` block has become fairly complex with many intermediate variables and nested conditionals; consider breaking out some of the fallback/merging logic into named helper filters or clearly separated blocks to make future maintenance and debugging easier.
## Individual Comments
### Comment 1
<location> `scripts/transform_plugin_data/transform_plugin_data/run.sh:3-7` </location>
<code_context>
-map(select(
- if .value.repo and ($repo_info[0][.value.repo]) then
- ($repo_info[0][.value.repo].status != "deleted")
+existing_cache_file="existing_cache.json"
+cleanup_existing_cache="false"
+
+# 如果没有历史缓存,为jq提供一个空对象以便统一逻辑
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Temporary cache file cleanup only happens on the happy path, which can leave stray files on errors.
`existing_cache_file` is created with `mktemp` but only removed when the script reaches the final cleanup. If `jq` or another command fails, the temporary file will be left behind. Consider adding `set -euo pipefail` and a `trap` to always clean up `existing_cache_file` on exit or error.
```suggestion
set -euo pipefail
# 始终在脚本退出时尝试清理临时缓存文件,避免错误情况下遗留
cleanup_existing_cache="false"
existing_cache_file="existing_cache.json"
cleanup() {
# 仅在当前执行中创建的临时缓存文件才会被清理
if [ "${cleanup_existing_cache}" = "true" ] && [ -n "${existing_cache_file:-}" ] && [ -f "${existing_cache_file}" ]; then
rm -f "${existing_cache_file}"
fi
}
trap cleanup EXIT INT TERM
echo "开始转换插件数据格式..."
# 使用jq转换数据格式,增加容错处理,并过滤掉404的仓库
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Soulter
approved these changes
Dec 9, 2025
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.
最近有些插件数据出现版本号回退到 1.0.0 的问题,所以有了这次提交。改动如下:把转换脚本改成在 GitHub API 失效时全面回退到缓存(覆盖 stars、version、updated_at、logo),并拒绝把已删除仓库重新写入,确保 CI 在 403 等异常下仍产出稳定数据。
Summary by Sourcery
改进插件数据转换脚本:当 GitHub API 响应不可用或出错时,依赖已有缓存数据,同时在重新生成输出时排除已删除或无效的仓库。
增强内容:
Original summary in English
Summary by Sourcery
Improve the plugin data transform script to rely on existing cache data when GitHub API responses are unavailable or erroneous, while excluding deleted or invalid repositories from regenerated output.
Enhancements: