docs: streamline project guidance and container CLI by IceCodeNew · Pull Request #81 · IceCodeNew/weather-briefing

IceCodeNew · 2026-07-21T05:24:22Z

Summary

focus the README on core features, deployment preparation, configuration, and operation
separate product requirements, technical design, and non-obvious tradeoffs into their owning documents
keep AGENTS.md concise and limited to reusable contributor guidance
use uv's venv console script directly for the image entrypoint and docker exec
preserve optional RSS setup and the release workflow version marker

Verification

prek run --all-files
uv run --with pytest --with pytest-cov -- pytest --cov --cov-branch --cov-report=xml (811 passed, 99% total coverage)
application code, tests, and dependency manifests are unchanged from master, so coverage does not regress
native image build with docker buildx build --load
final entrypoint: /home/nonroot/app/.venv/bin/weather-briefing as user 65532
default image entrypoint: weather-briefing 1.2.2-dev
real docker exec ... /home/nonroot/app/.venv/bin/weather-briefing --version: weather-briefing 1.2.2-dev
release workflow README version regex matches exactly once

coderabbitai · 2026-07-21T05:24:29Z

Warning

Review limit reached

@IceCodeNew, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 14f11aa5-1451-42c7-b620-44cc023405ac

📥 Commits

Reviewing files that changed from the base of the PR and between 7cda42f and 929631f.

📒 Files selected for processing (4)

AGENTS.md
Dockerfile
README.md
docs/design.md

📝 Walkthrough

Walkthrough

The PR rewrites product, system-design, trade-off, deployment, and agent workflow documentation. It also adds a Docker helper entrypoint, includes it in the build context, and changes the final image to execute the installed command.

Changes

Container runtime entrypoint

Layer / File(s)	Summary
Install and execute the container command `.dockerignore`, `docker/weather-briefing`, `Dockerfile`	The Docker context includes a Bash helper, the image installs it as `/usr/local/bin/weather-briefing`, and the final entrypoint executes it.

Project documentation model

Layer / File(s)	Summary
Define product behavior `docs/requirements.md`	Requirements now cover forecasts, change reminders, content, locations, warnings, reliability, privacy, and delivery.
Describe system contracts `docs/design.md`	System design now documents configuration, providers, language, RSS, LLM integration, scheduling, persistence, operations, and releases.
Record retained trade-offs `docs/notes.md`	Technical notes capture decisions about provider composition, compatibility, history, persistence, and logging.
Document deployment and operations `README.md`	README now documents Docker deployment, configuration, scheduling, manual runs, diagnostics, and logging.

Agent workflow guidance

Layer / File(s)	Summary
Update agent instructions `AGENTS.md`	Skills usage, documentation boundaries, engineering rules, tooling, Git workflow, review, verification, and pre-push guidance are condensed and reorganized.

Estimated code review effort: 2 (Simple) | ~15 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: documentation streamlining and the new container helper CLI.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/document-review-lessons

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

IceCodeNew · 2026-07-21T05:24:42Z

/agentic_review

qodo-code-review · 2026-07-21T05:25:56Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 37 rules

1. ~~Root-only chown instruction~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

README instructs chown -R 65532:65532 on the host without noting it typically requires root/sudo,
so the quickstart will fail for many non-root operators. If ownership/permissions aren’t corrected,
the non-root container may be unable to write the bind-mounted state/ directory and fail at
runtime.

Code

README.md[R38-46]

+```sh
+ROOT_DIR="${HOME}/weather-briefing"
+mkdir -p "${ROOT_DIR}/state"
+cp env.example "${ROOT_DIR}/.env"
+cp locations.example.json "${ROOT_DIR}/locations.json"
+printf '[]\n' >"${ROOT_DIR}/rss-sources.json"
+chmod 600 "${ROOT_DIR}/.env" "${ROOT_DIR}"/*.json
+chown -R 65532:65532 "${ROOT_DIR}"
+```

Relevance

⭐⭐ Medium
PR #40 accepted README chown step; no prior reviews noted needing sudo/root warning for such
instructions.
PR-#40

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

README’s quickstart includes a chown to UID/GID 65532. The Dockerfile shows the runtime image is
distroless :nonroot, uses /home/nonroot/app/ as WORKDIR, and declares /home/nonroot/app/state
as a volume, so the container expects to write state as that non-root identity—making host-side
permissions crucial.

README.md[38-66]
Dockerfile[22-39]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The README deployment snippet uses `chown -R 65532:65532 "${ROOT_DIR}"` but does not say that this generally requires root privileges, nor does it provide an alternative for non-root operators.

## Issue Context
The container runs as the distroless `nonroot` user (UID/GID 65532) and persists state under `/home/nonroot/app/state`, so the host bind mount must be writable by that UID.

## Fix Focus Areas
- README.md[38-46]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Helper path claim incorrect~~ ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New

Description

docs/design.md says both the container entrypoint and docker exec use
/usr/local/bin/weather-briefing, but the image PATH prepends the venv bin directory so `docker
exec ... weather-briefing resolves to /home/nonroot/app/.venv/bin/weather-briefing` instead. This
makes the design doc’s runtime contract inaccurate and can mislead operators about which launcher is
actually executed.

Code

docs/design.md[208]

+镜像提供 `/usr/local/bin/weather-briefing` helper。常驻入口和 `docker exec` 都通过它调用虚拟环境内 uv 生成的命令入口，调用方不需要激活虚拟环境。

Relevance

⭐⭐⭐ High
Team historically fixes docs when contract drifts; PR #32 and #59 updated docs to match runtime
behavior.
PR-#32
PR-#59

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The design doc explicitly claims docker exec uses /usr/local/bin/weather-briefing, but the
Dockerfile’s PATH places the venv bin directory first; therefore an unqualified weather-briefing
lookup will not use /usr/local/bin/weather-briefing.

docs/design.md[202-210]
Dockerfile[30-40]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`docs/design.md` claims both the container entrypoint and `docker exec` run via `/usr/local/bin/weather-briefing`, but the image PATH currently resolves `weather-briefing` to the venv-installed console script first.

## Issue Context
- The Dockerfile sets `PATH` to start with `/home/nonroot/app/.venv/bin`, so unqualified command lookups (including `docker exec weather-briefing ...`) will choose the venv script rather than `/usr/local/bin/weather-briefing`.
- The ENTRYPOINT is explicitly wired to `/usr/local/bin/weather-briefing`, so only the entrypoint reliably uses the helper.

## Fix Focus Areas
Choose one:
1) **Documentation-only fix**: Update `docs/design.md` to state that ENTRYPOINT uses the helper, while `docker exec weather-briefing ...` uses the venv console script (unless `/usr/local/bin/weather-briefing` is invoked explicitly).
2) **Behavior fix** (if you want `docker exec weather-briefing ...` to use the helper): Change PATH ordering so `/usr/local/bin` comes before the venv bin directory (and ensure this is intentional).

- docs/design.md[202-210]
- Dockerfile[34-40]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~RSS mount missing~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The README describes optional RSS sources but the Docker run example does not bind-mount
rss-sources.json (nor mention setting RSS_SOURCES_FILE), so following the example will run with
RSS silently disabled even if the operator prepares an RSS config on the host.

Code

README.md[R50-63]

+```sh
+WEATHER_BRIEFING_VERSION="1.2.1"
+IMAGE="icecodexi/weather-briefing:${WEATHER_BRIEFING_VERSION}"
+docker pull "${IMAGE}"
+docker run -d \
+  --name weather-briefing \
+  --restart unless-stopped \
+  --env-file "${ROOT_DIR}/.env" \
+  --mount \
+  "type=bind,src=${ROOT_DIR}/locations.json,dst=/home/nonroot/app/locations.json,readonly" \
+  --mount \
+  "type=bind,src=${ROOT_DIR}/state,dst=/home/nonroot/app/state" \
+  "${IMAGE}" daemon
+```

Relevance

⭐⭐⭐ High
Team previously accepted README Docker script fixes incl. rss-sources.json handling/mounting for
correct deployments (PR #40).
PR-#40

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The README’s Docker example mounts only locations.json and state/, while the implementation
defaults RSS_SOURCES_FILE to rss-sources.json relative to the container WORKDIR
(/home/nonroot/app). With no bind mount, the file will not exist in the container, and the config
loader treats that as an empty RSS feed list (RSS disabled).

README.md[26-63]
README.md[101-102]
weather_briefing/config.py[402-408]
Dockerfile[33-39]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`README.md` documents RSS as an optional feature, but the Docker deployment example never mounts `rss-sources.json` into the container. Because the app defaults `RSS_SOURCES_FILE` to `rss-sources.json` in the container working directory, RSS will remain disabled unless the operator manually adds a bind mount (or sets `RSS_SOURCES_FILE` to a mounted path).

### Issue Context
- Container WORKDIR is `/home/nonroot/app/`.
- Default `RSS_SOURCES_FILE` is `rss-sources.json` (relative to WORKDIR).
- Missing file is treated as “no RSS feeds”, so this failure mode is silent.

### Fix Focus Areas
- README.md[26-63]

### Suggested change
Update the README Docker section to clearly show how to enable RSS when desired, for example:
- Add a step like `cp rss-sources.example.json "${ROOT_DIR}/rss-sources.json"` (optional).
- Add an optional bind mount:
 - `--mount "type=bind,src=${ROOT_DIR}/rss-sources.json,dst=/home/nonroot/app/rss-sources.json,readonly"`
- Alternatively (or additionally) mention setting `RSS_SOURCES_FILE` to a mounted path if users prefer a different location.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Provider precedence ambiguous~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

README describes NEA/JMA as “优先”, which reads like they are the primary providers, but the system’s
default provider order is open-meteo first with local providers (nea-sg, jma-jp) appended as
capability supplements. This ambiguity can mislead operators about default execution/fallback order
and how to set WEATHER_PROVIDERS correctly.

Code

README.md[R82-90]

+程序会根据地点选择默认天气来源：

-```sh
-#!/bin/sh
-set -eu
-
-CONTAINER_NAME="weather-briefing"
-WEATHER_BRIEFING_IMAGE="icecodexi/${CONTAINER_NAME}"
-WEATHER_BRIEFING_VERSION="1.2.1"
-ROOT_DIR="${HOME}/${CONTAINER_NAME}"
-CONTAINER_ROOT_DIR="/home/nonroot/app"
-
-mkdir -p "${ROOT_DIR}/app/state"
-touch "${ROOT_DIR}/.env" "${ROOT_DIR}/locations.json"
-test -s "${ROOT_DIR}/rss-sources.json" || printf '[]\n' >"${ROOT_DIR}/rss-sources.json"
-chmod 600 "${ROOT_DIR}/.env" "${ROOT_DIR}/locations.json" "${ROOT_DIR}/rss-sources.json"
-chown -R 65532:65532 "${ROOT_DIR}"
+- 中国大陆：和风天气优先，Open-Meteo 备用；
+- 新加坡：NEA 两小时预报在对应时段优先，Open-Meteo 补齐其他数据；
+- 日本：配置 JMA office code 后，JMA 预报在对应范围内优先，Open-Meteo 补齐其他数据；
+- 其他地区：Open-Meteo。

-docker pull "${WEATHER_BRIEFING_IMAGE}:${WEATHER_BRIEFING_VERSION}"
-docker rm -f "${CONTAINER_NAME}" >/dev/null 2>&1 || true
-docker run -d \
-    --name "${CONTAINER_NAME}" \
-    --restart unless-stopped \
-    --env-file "${ROOT_DIR}/.env" \
-    --mount "type=bind,src=${ROOT_DIR}/locations.json,dst=${CONTAINER_ROOT_DIR}/locations.json,readonly" \
-    --mount "type=bind,src=${ROOT_DIR}/rss-sources.json,dst=${CONTAINER_ROOT_DIR}/rss-sources.json,readonly" \
-    --mount "type=bind,src=${ROOT_DIR}/app/state,dst=${CONTAINER_ROOT_DIR}/state" \
-    "${WEATHER_BRIEFING_IMAGE}:${WEATHER_BRIEFING_VERSION}" \
-    daemon
+也可以用 `WEATHER_PROVIDERS` 明确指定顺序。完整天气服务应排在只提供局部信息的服务之前。这个顺序决定程序如何取得完整数据，不代表后面的当地官方资料可信度较低。
+

Relevance

⭐⭐⭐ High
Team recently clarified provider-order semantics in docs/tests; PR #78 notes enforcing
capability-provider suffix and explaining ordering.
PR-#78
PR-#74
PR-#75

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

README’s region bullets say NEA/JMA are preferred, but the design doc and the runtime default
provider list show open-meteo is first and local providers are appended; the config code loads
those defaults. README’s later note about ordering partially clarifies it, which is why this is
primarily an ambiguity to fix in wording.

README.md[82-90]
docs/design.md[70-81]
weather_briefing/data/provider_defaults.json[1-18]
weather_briefing/config.py[230-245]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`README.md` says Singapore/Japan local providers are “优先”, which can be interpreted as the default provider *order*. In reality, the default order is Open-Meteo first with NEA/JMA appended, and local data may be treated as more authoritative only when overlapping/conflicting.

## Issue Context
- `docs/design.md` and `weather_briefing/data/provider_defaults.json` define the actual default order.
- `README.md` already explains that full weather providers should come before capability-only providers, but the region bullets conflict with that phrasing.

## Fix Focus Areas
- README.md[82-90]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. README includes uv lock --check ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

README.md adds a "本地运行" section with uv lock --check/uv sync --frozen, which is
development/contributor setup rather than user-facing usage guidance. This violates the requirement
to keep the root README focused on end-user installation/configuration/operation and link out
contributor details to separate docs.

Code

README.md[R132-142]

+## 本地运行
+
+项目支持 Python 3.11–3.14，并使用 uv 管理依赖：
+
+```sh
+uv lock --check
+uv sync --frozen
+cp env.example .env
+cp locations.example.json locations.json
+uv run --frozen weather-briefing run briefing
+```

Relevance

⭐ Low
Team historically keeps setup commands in README (deploy + uv-based usage). No evidence enforcing
“no dev setup in README”.
PR-#40
PR-#6

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2141666 requires the root README to contain only end-user usage information and not
include substantial contributor/development setup. The added "本地运行" section includes `uv lock
--check` and dependency sync steps, which are development workflow/setup commands rather than
user-facing operation instructions.

Rule 2141666: Restrict README.md content to user-facing usage information
README.md[132-142]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`README.md` contains contributor-focused local development setup commands (e.g., `uv lock --check`) instead of restricting the README to end-user usage information.

## Issue Context
The compliance rule requires the root README to avoid internal/contributor topics like detailed development setup, and to link to separate documents when such content is needed.

## Fix Focus Areas
- README.md[132-142]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Previous review results

Review updated until commit 929631f

Results up to commit 12286b8 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. README includes uv lock --check ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

README.md adds a "本地运行" section with uv lock --check/uv sync --frozen, which is
development/contributor setup rather than user-facing usage guidance. This violates the requirement
to keep the root README focused on end-user installation/configuration/operation and link out
contributor details to separate docs.

Code

README.md[R132-142]

+## 本地运行
+
+项目支持 Python 3.11–3.14，并使用 uv 管理依赖：
+
+```sh
+uv lock --check
+uv sync --frozen
+cp env.example .env
+cp locations.example.json locations.json
+uv run --frozen weather-briefing run briefing
+```

Relevance

⭐ Low
Team historically keeps setup commands in README (deploy + uv-based usage). No evidence enforcing
“no dev setup in README”.
PR-#40
PR-#6

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2141666 requires the root README to contain only end-user usage information and not
include substantial contributor/development setup. The added "本地运行" section includes `uv lock
--check` and dependency sync steps, which are development workflow/setup commands rather than
user-facing operation instructions.

Rule 2141666: Restrict README.md content to user-facing usage information
README.md[132-142]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`README.md` contains contributor-focused local development setup commands (e.g., `uv lock --check`) instead of restricting the README to end-user usage information.

## Issue Context
The compliance rule requires the root README to avoid internal/contributor topics like detailed development setup, and to link to separate documents when such content is needed.

## Fix Focus Areas
- README.md[132-142]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Results up to commit a734a40 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Root-only chown instruction~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

README instructs chown -R 65532:65532 on the host without noting it typically requires root/sudo,
so the quickstart will fail for many non-root operators. If ownership/permissions aren’t corrected,
the non-root container may be unable to write the bind-mounted state/ directory and fail at
runtime.

Code

README.md[R38-46]

+```sh
+ROOT_DIR="${HOME}/weather-briefing"
+mkdir -p "${ROOT_DIR}/state"
+cp env.example "${ROOT_DIR}/.env"
+cp locations.example.json "${ROOT_DIR}/locations.json"
+printf '[]\n' >"${ROOT_DIR}/rss-sources.json"
+chmod 600 "${ROOT_DIR}/.env" "${ROOT_DIR}"/*.json
+chown -R 65532:65532 "${ROOT_DIR}"
+```

Relevance

⭐⭐ Medium
PR #40 accepted README chown step; no prior reviews noted needing sudo/root warning for such
instructions.
PR-#40

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

README’s quickstart includes a chown to UID/GID 65532. The Dockerfile shows the runtime image is
distroless :nonroot, uses /home/nonroot/app/ as WORKDIR, and declares /home/nonroot/app/state
as a volume, so the container expects to write state as that non-root identity—making host-side
permissions crucial.

README.md[38-66]
Dockerfile[22-39]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The README deployment snippet uses `chown -R 65532:65532 "${ROOT_DIR}"` but does not say that this generally requires root privileges, nor does it provide an alternative for non-root operators.

## Issue Context
The container runs as the distroless `nonroot` user (UID/GID 65532) and persists state under `/home/nonroot/app/state`, so the host bind mount must be writable by that UID.

## Fix Focus Areas
- README.md[38-46]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Provider precedence ambiguous~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

README describes NEA/JMA as “优先”, which reads like they are the primary providers, but the system’s
default provider order is open-meteo first with local providers (nea-sg, jma-jp) appended as
capability supplements. This ambiguity can mislead operators about default execution/fallback order
and how to set WEATHER_PROVIDERS correctly.

Code

README.md[R82-90]

+程序会根据地点选择默认天气来源：

-```sh
-#!/bin/sh
-set -eu
-
-CONTAINER_NAME="weather-briefing"
-WEATHER_BRIEFING_IMAGE="icecodexi/${CONTAINER_NAME}"
-WEATHER_BRIEFING_VERSION="1.2.1"
-ROOT_DIR="${HOME}/${CONTAINER_NAME}"
-CONTAINER_ROOT_DIR="/home/nonroot/app"
-
-mkdir -p "${ROOT_DIR}/app/state"
-touch "${ROOT_DIR}/.env" "${ROOT_DIR}/locations.json"
-test -s "${ROOT_DIR}/rss-sources.json" || printf '[]\n' >"${ROOT_DIR}/rss-sources.json"
-chmod 600 "${ROOT_DIR}/.env" "${ROOT_DIR}/locations.json" "${ROOT_DIR}/rss-sources.json"
-chown -R 65532:65532 "${ROOT_DIR}"
+- 中国大陆：和风天气优先，Open-Meteo 备用；
+- 新加坡：NEA 两小时预报在对应时段优先，Open-Meteo 补齐其他数据；
+- 日本：配置 JMA office code 后，JMA 预报在对应范围内优先，Open-Meteo 补齐其他数据；
+- 其他地区：Open-Meteo。

-docker pull "${WEATHER_BRIEFING_IMAGE}:${WEATHER_BRIEFING_VERSION}"
-docker rm -f "${CONTAINER_NAME}" >/dev/null 2>&1 || true
-docker run -d \
-    --name "${CONTAINER_NAME}" \
-    --restart unless-stopped \
-    --env-file "${ROOT_DIR}/.env" \
-    --mount "type=bind,src=${ROOT_DIR}/locations.json,dst=${CONTAINER_ROOT_DIR}/locations.json,readonly" \
-    --mount "type=bind,src=${ROOT_DIR}/rss-sources.json,dst=${CONTAINER_ROOT_DIR}/rss-sources.json,readonly" \
-    --mount "type=bind,src=${ROOT_DIR}/app/state,dst=${CONTAINER_ROOT_DIR}/state" \
-    "${WEATHER_BRIEFING_IMAGE}:${WEATHER_BRIEFING_VERSION}" \
-    daemon
+也可以用 `WEATHER_PROVIDERS` 明确指定顺序。完整天气服务应排在只提供局部信息的服务之前。这个顺序决定程序如何取得完整数据，不代表后面的当地官方资料可信度较低。
+

Relevance

⭐⭐⭐ High
Team recently clarified provider-order semantics in docs/tests; PR #78 notes enforcing
capability-provider suffix and explaining ordering.
PR-#78
PR-#74
PR-#75

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

README’s region bullets say NEA/JMA are preferred, but the design doc and the runtime default
provider list show open-meteo is first and local providers are appended; the config code loads
those defaults. README’s later note about ordering partially clarifies it, which is why this is
primarily an ambiguity to fix in wording.

README.md[82-90]
docs/design.md[70-81]
weather_briefing/data/provider_defaults.json[1-18]
weather_briefing/config.py[230-245]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`README.md` says Singapore/Japan local providers are “优先”, which can be interpreted as the default provider *order*. In reality, the default order is Open-Meteo first with NEA/JMA appended, and local data may be treated as more authoritative only when overlapping/conflicting.

## Issue Context
- `docs/design.md` and `weather_briefing/data/provider_defaults.json` define the actual default order.
- `README.md` already explains that full weather providers should come before capability-only providers, but the region bullets conflict with that phrasing.

## Fix Focus Areas
- README.md[82-90]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Results up to commit 028063e ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~RSS mount missing~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The README describes optional RSS sources but the Docker run example does not bind-mount
rss-sources.json (nor mention setting RSS_SOURCES_FILE), so following the example will run with
RSS silently disabled even if the operator prepares an RSS config on the host.

Code

README.md[R50-63]

+```sh
+WEATHER_BRIEFING_VERSION="1.2.1"
+IMAGE="icecodexi/weather-briefing:${WEATHER_BRIEFING_VERSION}"
+docker pull "${IMAGE}"
+docker run -d \
+  --name weather-briefing \
+  --restart unless-stopped \
+  --env-file "${ROOT_DIR}/.env" \
+  --mount \
+  "type=bind,src=${ROOT_DIR}/locations.json,dst=/home/nonroot/app/locations.json,readonly" \
+  --mount \
+  "type=bind,src=${ROOT_DIR}/state,dst=/home/nonroot/app/state" \
+  "${IMAGE}" daemon
+```

Relevance

⭐⭐⭐ High
Team previously accepted README Docker script fixes incl. rss-sources.json handling/mounting for
correct deployments (PR #40).
PR-#40

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The README’s Docker example mounts only locations.json and state/, while the implementation
defaults RSS_SOURCES_FILE to rss-sources.json relative to the container WORKDIR
(/home/nonroot/app). With no bind mount, the file will not exist in the container, and the config
loader treats that as an empty RSS feed list (RSS disabled).

README.md[26-63]
README.md[101-102]
weather_briefing/config.py[402-408]
Dockerfile[33-39]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`README.md` documents RSS as an optional feature, but the Docker deployment example never mounts `rss-sources.json` into the container. Because the app defaults `RSS_SOURCES_FILE` to `rss-sources.json` in the container working directory, RSS will remain disabled unless the operator manually adds a bind mount (or sets `RSS_SOURCES_FILE` to a mounted path).

### Issue Context
- Container WORKDIR is `/home/nonroot/app/`.
- Default `RSS_SOURCES_FILE` is `rss-sources.json` (relative to WORKDIR).
- Missing file is treated as “no RSS feeds”, so this failure mode is silent.

### Fix Focus Areas
- README.md[26-63]

### Suggested change
Update the README Docker section to clearly show how to enable RSS when desired, for example:
- Add a step like `cp rss-sources.example.json "${ROOT_DIR}/rss-sources.json"` (optional).
- Add an optional bind mount:
 - `--mount "type=bind,src=${ROOT_DIR}/rss-sources.json,dst=/home/nonroot/app/rss-sources.json,readonly"`
- Alternatively (or additionally) mention setting `RSS_SOURCES_FILE` to a mounted path if users prefer a different location.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

IceCodeNew · 2026-07-21T05:53:04Z

/agentic_review

qodo-code-review · 2026-07-21T05:58:08Z

Code review by qodo was updated up to the latest commit 12286b8

IceCodeNew · 2026-07-21T06:03:53Z

/agentic_review

qodo-code-review · 2026-07-21T06:08:58Z

Code review by qodo was updated up to the latest commit a734a40

IceCodeNew · 2026-07-21T06:13:41Z

/agentic_review

qodo-code-review · 2026-07-21T06:18:25Z

Code review by qodo was updated up to the latest commit f638296

qodo-code-review · 2026-07-21T06:23:53Z

PR Summary by Qodo

Docs: clarify requirements/design/notes; add Docker weather-briefing helper

📝 Documentation ✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Refocus README on core capabilities, deployment prerequisites, configuration, and operations.
• Separate product requirements, current technical contract, and tradeoff notes across docs/.
• Add a container entrypoint helper and update Docker build context allowlist.

Diagram

graph TD
  A["Operator"] --> B["README.md"] --> C["Docker image"] --> D["/usr/local/bin/weather-briefing"] --> E["weather_briefing CLI"]
  A --> F["docs/* (requirements/design/notes)"]
  A --> G["AGENTS.md"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep Python module ENTRYPOINT

➕ Fewer moving parts (no extra wrapper script to ship).
➕ ENTRYPOINT remains self-describing (python -m weather_briefing.cli).
➖ Requires keeping PATH/venv invocation details correct in ENTRYPOINT.
➖ Harder to reuse the exact same invocation pattern for docker exec vs entrypoint.

2. Use installed console script directly via PATH (no wrapper)

➕ Avoids adding a custom script to the image.
➕ Leverages the packaging-generated entrypoint.
➖ Depends on PATH correctness and where uv installs scripts in the image layout.
➖ Less explicit about always executing the venv-scoped binary.

Recommendation: The chosen wrapper-helper approach is reasonable for distroless-style images: it makes the container entrypoint and docker exec behavior consistent and guarantees the venv CLI is used. The only follow-up to consider is ensuring the helper uses a maximally portable shebang for the base image (and documenting it if bash is a deliberate requirement).

Files changed (8) +489 / -397

Documentation (5) +481 / -396

AGENTS.mdClarify doc ownership, review workflow, and commit guidance +97/-60

Clarify doc ownership, review workflow, and commit guidance

• Rewrites guidance to clearly separate README vs requirements vs design vs notes responsibilities. Expands contributor workflow rules around review sequencing, verification gates, and commit/stacking practices.

AGENTS.md

README.mdRefocus README on capabilities and deployment/operations +108/-97

Refocus README on capabilities and deployment/operations

• Replaces the long implementation-heavy overview with a concise capability list and a deployment-first structure. Adds clearer prerequisites, Docker run examples, location/model configuration guidance, and operational troubleshooting notes, with pointers to requirements/design/notes for deeper detail.

README.md

design.mdRewrite design doc as current technical contract +126/-92

Rewrite design doc as current technical contract

• Reframes the document as the authoritative description of the current system boundaries, configuration inputs, and provider composition rules. Organizes content into clear sections (flow, configuration, capabilities, language, scheduling, state, logging/privacy, build/release) and reduces historical narration.

docs/design.md

notes.mdRescope notes to non-obvious tradeoffs and reevaluation triggers +60/-90

Rescope notes to non-obvious tradeoffs and reevaluation triggers

• Converts the notes doc into a curated set of explicit tradeoffs with assumptions and triggers for revisiting decisions. Removes routine design repetition and keeps only items that may look questionable without extra context.

docs/notes.md

requirements.mdRewrite requirements as implementation-neutral product behavior +90/-57

Rewrite requirements as implementation-neutral product behavior

• Restructures the document into product goals, daily forecast behavior, change-notification behavior, content rules, location/region behavior, reliability expectations, and operational/privacy constraints. Removes detailed implementation and internal component naming, and links to README/design for those concerns.

docs/requirements.md

Other (3) +8 / -1

.dockerignoreAllow Docker context to include docker/ helper assets +2/-0

Allow Docker context to include docker/ helper assets

• Adds allowlist entries for the docker/ directory and the new docker/weather-briefing helper so BuildKit can copy it into the final image. Preserves the existing whitelist-based Docker context strategy.

.dockerignore

DockerfileSwitch container ENTRYPOINT to helper wrapper +2/-1

Switch container ENTRYPOINT to helper wrapper

• Copies the new docker/weather-briefing script into /usr/local/bin and uses it as the container ENTRYPOINT. This makes the container start command invoke the venv-installed CLI consistently.

Dockerfile

weather-briefingAdd container helper to run venv weather-briefing CLI +4/-0

Add container helper to run venv weather-briefing CLI

• Introduces a small wrapper script that execs the virtualenv-installed weather-briefing entrypoint. Intended to be used both as the container ENTRYPOINT and for consistent invocation via docker exec.

docker/weather-briefing

qodo-code-review · 2026-07-21T06:27:47Z

Code review by qodo was updated up to the latest commit 13d35c7

IceCodeNew · 2026-07-21T06:53:08Z

/agentic_review

qodo-code-review · 2026-07-21T06:56:23Z

Code review by qodo was updated up to the latest commit d103a6d

qodo-code-review · 2026-07-21T07:02:12Z

Code review by qodo was updated up to the latest commit d103a6d

IceCodeNew · 2026-07-21T07:23:28Z

@coderabbitai review

coderabbitai · 2026-07-21T07:23:34Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (1)

AGENTS.md (1)
123-127: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Drop the --with flags here. pytest and pytest-cov are already pinned in uv.lock, and uv run pytest --cov --cov-branch --cov-report=xml matches the workflow command more closely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@AGENTS.md` around lines 123 - 127, Update the pre-push test command in
AGENTS.md to remove both --with pytest and --with pytest-cov, using uv run
pytest --cov --cov-branch --cov-report=xml while preserving the existing hook
command.
Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/design.md`:
- Around line 182-201: Update the RSS retry handling documented around the
“失败与告警” section and its implementation in the Retry-After parser to support both
valid forms: delta-seconds and HTTP-date values. Preserve the existing retry
delay behavior for valid HTTP dates, and apply valid numeric delays instead of
ignoring them; only narrow the documentation if the implementation is
intentionally not expanded.

In `@README.md`:
- Around line 40-48: Reorder the README setup commands so users edit `.env` and
`locations.json` before running `sudo chown -R 65532:65532 "${ROOT_DIR}"` and
the restrictive chmod command. Keep the final ownership and permissions intact
while ensuring the documented configuration step remains writable by the host
user.
- Around line 128-137: Update the rendered-text diagnostics section in README.md
to instruct users to set DEBUG=true before starting or restarting the
weather-briefing container, then run the existing enable command; clarify that
enabling the persisted switch alone does not affect a daemon that already loaded
Settings.from_env().

---

Nitpick comments:
In `@AGENTS.md`:
- Around line 123-127: Update the pre-push test command in AGENTS.md to remove
both --with pytest and --with pytest-cov, using uv run pytest --cov --cov-branch
--cov-report=xml while preserving the existing hook command.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2214a752-d622-4b26-a2e9-6411e530d1ba

📥 Commits

Reviewing files that changed from the base of the PR and between 01fd83e and 7cda42f.

📒 Files selected for processing (8)

.dockerignore
AGENTS.md
Dockerfile
README.md
docker/weather-briefing
docs/design.md
docs/notes.md
docs/requirements.md

IceCodeNew · 2026-07-21T07:37:27Z

/agentic_review

qodo-code-review · 2026-07-21T07:41:44Z

Code review by qodo was updated up to the latest commit 8223c32

IceCodeNew · 2026-07-21T07:49:13Z

/agentic_review

qodo-code-review · 2026-07-21T07:51:18Z

Code review by qodo was updated up to the latest commit 929631f

docs: document pull request review lessons

8880aec

IceCodeNew added 6 commits July 21, 2026 13:50

docs: focus readme on deployment

15b32b5

docs: separate product requirements from design

4f9ffaa

docs: define the current system design

6d2eae2

docs: keep notes for non-obvious tradeoffs

3465c56

docs: clarify documentation and commit guidance

f21db9d

docs: highlight core user capabilities

12286b8

IceCodeNew changed the title ~~docs: document pull request review lessons~~ docs: clarify project documentation Jul 21, 2026

IceCodeNew added 2 commits July 21, 2026 14:01

docs: clarify location and regional source guidance

74f6bec

docs: replace stable id terminology

a734a40

docs: keep requirements implementation neutral

95d0a3b

qodo-code-review Bot reviewed Jul 21, 2026

View reviewed changes

Comment thread README.md

Comment thread README.md

IceCodeNew added 2 commits July 21, 2026 14:11

docs: clarify deployment and provider precedence

bceedeb

docs: explain legacy DeepSeek configuration

f638296

docs: remove routine sdk retry rationale

c8f50b6

docs: split long paragraphs by topic

13d35c7

IceCodeNew marked this pull request as ready for review July 21, 2026 06:22

qodo-code-review Bot added the 🕐 40+ Minutes label Jul 21, 2026

IceCodeNew marked this pull request as draft July 21, 2026 06:24

IceCodeNew marked this pull request as ready for review July 21, 2026 06:56

IceCodeNew changed the title ~~docs: clarify project documentation~~ docs: restructure project guidance and add container helper Jul 21, 2026

qodo-code-review Bot reviewed Jul 21, 2026

View reviewed changes

Comment thread docs/design.md Outdated

docs: simplify contributor guidance

7cda42f

IceCodeNew changed the title ~~docs: restructure project guidance and add container helper~~ docs: streamline project guidance and container CLI Jul 21, 2026

IceCodeNew requested a review from Copilot July 21, 2026 07:29

Copilot AI reviewed Jul 21, 2026

coderabbitai Bot reviewed Jul 21, 2026

View reviewed changes

Comment thread docs/design.md

Comment thread README.md Outdated

Comment thread README.md Outdated

IceCodeNew added 2 commits July 21, 2026 15:30

Update AGENTS.md

5b78d99

docs: address deployment review feedback

8223c32

docs: keep explicit test dependencies

47aa485

refactor(container): use venv entry point directly

929631f

IceCodeNew merged commit fdb1b7b into master Jul 21, 2026
6 of 7 checks passed

IceCodeNew deleted the codex/document-review-lessons branch July 21, 2026 07:55

qodo-code-review Bot mentioned this pull request Jul 30, 2026

refactor(delivery): migrate Telegram to Apprise #127

Merged

Conversation

IceCodeNew commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Uh oh!

coderabbitai Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

IceCodeNew commented Jul 21, 2026

Uh oh!

qodo-code-review Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

IceCodeNew commented Jul 21, 2026

Uh oh!

qodo-code-review Bot commented Jul 21, 2026

Uh oh!

IceCodeNew commented Jul 21, 2026

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 21, 2026

Uh oh!

IceCodeNew commented Jul 21, 2026

Uh oh!

qodo-code-review Bot commented Jul 21, 2026

Uh oh!

qodo-code-review Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Jul 21, 2026

Uh oh!

IceCodeNew commented Jul 21, 2026

Uh oh!

qodo-code-review Bot commented Jul 21, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 21, 2026

Uh oh!

IceCodeNew commented Jul 21, 2026

Uh oh!

coderabbitai Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IceCodeNew commented Jul 21, 2026

Uh oh!

qodo-code-review Bot commented Jul 21, 2026

Uh oh!

IceCodeNew commented Jul 21, 2026

Uh oh!

qodo-code-review Bot commented Jul 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

IceCodeNew commented Jul 21, 2026 •

edited

Loading

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

qodo-code-review Bot commented Jul 21, 2026 •

edited

Loading

qodo-code-review Bot commented Jul 21, 2026 •

edited

Loading

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading