Skip to content

fix: suppress sensitive SDK debug logs - #55

Merged
IceCodeNew merged 1 commit into
masterfrom
codex/protect-debug-logs
Jul 17, 2026
Merged

fix: suppress sensitive SDK debug logs#55
IceCodeNew merged 1 commit into
masterfrom
codex/protect-debug-logs

Conversation

@IceCodeNew

Copy link
Copy Markdown
Owner

Summary

  • keep application DEBUG diagnostics available on the weather_briefing logger
  • hold root and third-party SDK loggers at WARNING
  • prevent any-llm/OpenAI clients from dumping prompts, locations, feed content, and request bodies
  • document the logging boundary and test both allowed metadata and suppressed payloads

Root cause

Enabling application DEBUG also raised the root logger to DEBUG, allowing the OpenAI SDK to serialize the complete LLM request into production logs.

Validation

  • prek run --all-files
  • uv run --with pytest --with pytest-cov -- pytest --cov --cov-branch --cov-report=xml (594 passed)

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 12 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: de037de4-ee88-4410-8269-08c4b62d4609

📥 Commits

Reviewing files that changed from the base of the PR and between beb7c3d and 2ad6cba.

📒 Files selected for processing (7)
  • README.md
  • docs/design.md
  • docs/notes.md
  • tests/test_cli.py
  • tests/test_llm.py
  • weather_briefing/cli.py
  • weather_briefing/llm.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/protect-debug-logs

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.

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.73%. Comparing base (beb7c3d) to head (2ad6cba).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #55   +/-   ##
=======================================
  Coverage   99.72%   99.73%           
=======================================
  Files          39       39           
  Lines        6643     6739   +96     
  Branches      378      385    +7     
=======================================
+ Hits         6625     6721   +96     
  Misses         13       13           
  Partials        5        5           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@qodo-code-review

qodo-code-review Bot commented Jul 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 31 rules

Grey Divider


Remediation recommended

1. Duplicate diagnostics state reads ✓ Resolved 🐞 Bug ➹ Performance ⭐ New
Description
AnyLLMStructuredProvider.summarize() calls _sensitive_llm_diagnostics_enabled() twice (request +
response), which can re-query the runtime diagnostics state twice per LLM call when backed by
SQLiteRuntimeDiagnostics. This is avoidable extra work because
SQLiteRuntimeDiagnostics.rendered_text_logging_enabled() performs SQL reads (and may delete expired
state), so the second check repeats the same operation in the common case.
Code

weather_briefing/llm.py[R190-198]

+        result_payload = result.model_dump(mode="json")
+        if _sensitive_llm_diagnostics_enabled(self._diagnostics):
+            _LOGGER.debug(
+                "Sensitive LLM response diagnostic: provider=%s model=%s payload=%r",
+                self._provider,
+                self._model,
+                result_payload,
+            )
+        return result_payload
Relevance

⭐⭐ Medium

No prior precedent for caching diagnostics flag; team optimizes SQLite/state paths elsewhere (PRs
#34, #17).

PR-#34
PR-#17
PR-#39

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The LLM adapter gates both request and response diagnostics with separate calls to
_sensitive_llm_diagnostics_enabled(), so it can query the diagnostics switch twice per
summarize() call. When the diagnostics implementation is SQLiteRuntimeDiagnostics,
rendered_text_logging_enabled() calls rendered_text_logging_until() which executes a SQL
SELECT and may also DELETE and COMMIT expired state, making duplicate checks tangible extra
database work.

weather_briefing/llm.py[165-199]
weather_briefing/state.py[28-33]
weather_briefing/state.py[70-102]

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

### Issue description
`AnyLLMStructuredProvider.summarize()` checks `_sensitive_llm_diagnostics_enabled(self._diagnostics)` twice (before logging the request and again before logging the response). When `diagnostics` is a `SQLiteRuntimeDiagnostics`, each check can execute SQL (and potentially a DELETE+COMMIT on expiry), so the second check repeats work during a single `summarize()`.

### Issue Context
- This path is only active when DEBUG logging is enabled and a diagnostics object is provided.
- `SQLiteRuntimeDiagnostics.rendered_text_logging_enabled()` calls into `rendered_text_logging_until()`, which issues a `SELECT` and may `DELETE` and `COMMIT` expired state.

### Fix Focus Areas
- weather_briefing/llm.py[165-199]
- weather_briefing/state.py[28-33]
- weather_briefing/state.py[70-102]

### Suggested fix
In `summarize()`, compute a boolean once per call, e.g.:

```py
log_sensitive = _sensitive_llm_diagnostics_enabled(self._diagnostics)
if log_sensitive:
   ... log request ...
...
if log_sensitive:
   ... log response ...
```

This keeps request/response diagnostics consistent within one LLM call and avoids a second SQLite-backed state lookup.

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



Informational

2. Duplicated SDK logger list ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
tests/test_cli.py hard-codes the sensitive SDK logger names in multiple places instead of asserting
against a single contract, increasing the chance that future changes to the protected-logger
boundary become partially untested. This is a maintainability/coverage risk (especially when adding
new sensitive SDK namespaces).
Code

tests/test_cli.py[R51-52]

+    sdk_loggers = [logging.getLogger(name) for name in ("any_llm", "openai", "httpx", "httpcore")]
+    original_sdk_levels = [logger.level for logger in sdk_loggers]
Relevance

⭐⭐⭐ High

Team previously refactored tests to remove duplicated literals via shared fixtures/constants (PR
#14).

PR-#14

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Production defines a single sensitive-logger tuple used by _configure_logging, but tests duplicate
the logger-name list in multiple places, which can drift as the logging boundary evolves.

weather_briefing/cli.py[159-199]
tests/test_cli.py[45-118]

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

### Issue description
`tests/test_cli.py` duplicates the sensitive SDK logger-name list (e.g., `("any_llm", "openai", "httpx", "httpcore")`) in multiple tests, while production uses `weather_briefing.cli._SENSITIVE_SDK_LOGGERS`. This duplication can drift over time, reducing confidence that changes to the logging boundary are fully covered.

### Issue Context
- Production behavior is controlled by `_SENSITIVE_SDK_LOGGERS` in `weather_briefing/cli.py`.
- Tests currently re-state the same names manually.

### Fix Focus Areas
- tests/test_cli.py[45-118]
- weather_briefing/cli.py[159-199]

### Suggested fix
Choose one of these patterns:
1) Keep an *independent* expected set in tests (to preserve security intent), and add an assertion that production `_SENSITIVE_SDK_LOGGERS` contains at least those required names.
2) Or expose a small public constant (e.g., `weather_briefing.logging_policy.SENSITIVE_SDK_LOGGERS`) and reference it from both production and tests, plus add a separate test asserting it includes the required namespaces.

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


3. docs/notes.md missing logging rationale ✓ Resolved 📘 Rule violation ⌂ Architecture
Description
The code change introduces an architectural logging decision (root and sensitive SDK loggers forced
to WARNING while app logger can be DEBUG) without adding a corresponding
rationale/trade-offs/boundaries entry in docs/notes.md. This violates the requirement to document
non-obvious architectural decisions in docs/notes.md.
Code

weather_briefing/cli.py[R194-198]

+    logging.root.setLevel(logging.WARNING)
+    for handler in logging.root.handlers:
+        handler.setLevel(logging.WARNING)
+    for logger_name in _SENSITIVE_SDK_LOGGERS:
+        logging.getLogger(logger_name).setLevel(logging.WARNING)
Relevance

⭐ Low

Logging boundaries are documented in docs/design.md historically; docs/notes.md used for bigger
architecture (PR #37).

PR-#17
PR-#21
PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The implementation now enforces logging.root and a set of sensitive SDK loggers at WARNING,
which is a cross-cutting architectural boundary meant to prevent sensitive payload leakage.
docs/notes.md contains logging-related notes (e.g., two-stage logging init) but does not document
this new boundary’s rationale/trade-offs/assumptions as required.

Rule 2141673: Document non-obvious architectural decisions in docs/notes.md
weather_briefing/cli.py[159-198]
docs/notes.md[9-27]

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

## Issue description
A non-obvious architectural decision was added/changed in logging behavior (forcing root and sensitive SDK loggers to `WARNING`), but `docs/notes.md` was not updated with the required decision+rationale+trade-offs+operating boundaries.

## Issue Context
`docs/design.md` describes the mechanics, but PR Compliance ID 2141673 specifically requires non-obvious architectural decisions to be recorded in `docs/notes.md` with rationale, trade-offs, and operating boundaries/assumptions.

## Fix Focus Areas
- weather_briefing/cli.py[159-198]
- docs/notes.md[5-27]

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


4. README logs section too internal ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The updated README adds detailed internal logging-boundary behavior (e.g., third-party SDK logger
level policy) that is more appropriate for design/ops docs than end-user usage guidance. This
violates the requirement that root README.md be limited to user-facing usage information, with
internal details linked out to separate documents.
Code

README.md[48]

+应用将带时间、级别和 logger 名称的运行日志写入标准错误;INFO 日志记录每个地点的天气 provider 顺序和逻辑降级过程,并为天气、空气质量、地理编码、LLM、RSS、辅助上下文及 Telegram 的每个实际 HTTP 请求记录 provider、operation、方法、成功或失败、耗时和 HTTP 状态或异常类型,因此可从容器日志还原外部 API 调用历史。RSS 重试与 Telegram 分片分别按实际请求次数记录。常规 INFO 日志及仅由 `DEBUG=true` 启用的非敏感诊断不记录坐标、标题、正文、URL、token、chat ID、请求 endpoint 或异常消息;DEBUG 仅作用于应用 logger,any-llm、OpenAI、HTTPX 等第三方 SDK 保持 WARNING,避免其输出完整请求。应用 DEBUG 元数据覆盖从 RSS 清洗、权威预报转发和平台渲染到 Telegram 分片接受状态的链路。若仍需排查平台渲染或分片内容,可在不重启 daemon 的情况下临时记录完整渲染正文:
Relevance

⭐ Low

README already contains ops/logging details and was expanded similarly in PRs #17/#21/#40.

PR-#17
PR-#21
PR-#40

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2141666 requires the root README to avoid maintainer/internal operational detail.
The changed README paragraph now documents internal logging policy boundaries for third-party SDKs
as part of the README body.

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

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 expanded internal/maintainer-focused logging boundary details.

## Issue Context
PR Compliance ID 2141666 requires the root README to remain user-facing (install/config/run), and to link out to separate docs for internal architecture/operations detail.

## Fix Focus Areas
- README.md[46-56]
- docs/design.md[129-136]

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


Grey Divider

Previous review results

Review updated until commit 2ad6cba

Results up to commit 5fcfd0f


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


Informational
1. docs/notes.md missing logging rationale ✓ Resolved 📘 Rule violation ⌂ Architecture
Description
The code change introduces an architectural logging decision (root and sensitive SDK loggers forced
to WARNING while app logger can be DEBUG) without adding a corresponding
rationale/trade-offs/boundaries entry in docs/notes.md. This violates the requirement to document
non-obvious architectural decisions in docs/notes.md.
Code

weather_briefing/cli.py[R194-198]

+    logging.root.setLevel(logging.WARNING)
+    for handler in logging.root.handlers:
+        handler.setLevel(logging.WARNING)
+    for logger_name in _SENSITIVE_SDK_LOGGERS:
+        logging.getLogger(logger_name).setLevel(logging.WARNING)
Relevance

⭐ Low

Logging boundaries are documented in docs/design.md historically; docs/notes.md used for bigger
architecture (PR #37).

PR-#17
PR-#21
PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The implementation now enforces logging.root and a set of sensitive SDK loggers at WARNING,
which is a cross-cutting architectural boundary meant to prevent sensitive payload leakage.
docs/notes.md contains logging-related notes (e.g., two-stage logging init) but does not document
this new boundary’s rationale/trade-offs/assumptions as required.

Rule 2141673: Document non-obvious architectural decisions in docs/notes.md
weather_briefing/cli.py[159-198]
docs/notes.md[9-27]

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

## Issue description
A non-obvious architectural decision was added/changed in logging behavior (forcing root and sensitive SDK loggers to `WARNING`), but `docs/notes.md` was not updated with the required decision+rationale+trade-offs+operating boundaries.

## Issue Context
`docs/design.md` describes the mechanics, but PR Compliance ID 2141673 specifically requires non-obvious architectural decisions to be recorded in `docs/notes.md` with rationale, trade-offs, and operating boundaries/assumptions.

## Fix Focus Areas
- weather_briefing/cli.py[159-198]
- docs/notes.md[5-27]

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


2. README logs section too internal ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The updated README adds detailed internal logging-boundary behavior (e.g., third-party SDK logger
level policy) that is more appropriate for design/ops docs than end-user usage guidance. This
violates the requirement that root README.md be limited to user-facing usage information, with
internal details linked out to separate documents.
Code

README.md[48]

+应用将带时间、级别和 logger 名称的运行日志写入标准错误;INFO 日志记录每个地点的天气 provider 顺序和逻辑降级过程,并为天气、空气质量、地理编码、LLM、RSS、辅助上下文及 Telegram 的每个实际 HTTP 请求记录 provider、operation、方法、成功或失败、耗时和 HTTP 状态或异常类型,因此可从容器日志还原外部 API 调用历史。RSS 重试与 Telegram 分片分别按实际请求次数记录。常规 INFO 日志及仅由 `DEBUG=true` 启用的非敏感诊断不记录坐标、标题、正文、URL、token、chat ID、请求 endpoint 或异常消息;DEBUG 仅作用于应用 logger,any-llm、OpenAI、HTTPX 等第三方 SDK 保持 WARNING,避免其输出完整请求。应用 DEBUG 元数据覆盖从 RSS 清洗、权威预报转发和平台渲染到 Telegram 分片接受状态的链路。若仍需排查平台渲染或分片内容,可在不重启 daemon 的情况下临时记录完整渲染正文:
Relevance

⭐ Low

README already contains ops/logging details and was expanded similarly in PRs #17/#21/#40.

PR-#17
PR-#21
PR-#40

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2141666 requires the root README to avoid maintainer/internal operational detail.
The changed README paragraph now documents internal logging policy boundaries for third-party SDKs
as part of the README body.

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

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 expanded internal/maintainer-focused logging boundary details.

## Issue Context
PR Compliance ID 2141666 requires the root README to remain user-facing (install/config/run), and to link out to separate docs for internal architecture/operations detail.

## Fix Focus Areas
- README.md[46-56]
- docs/design.md[129-136]

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


Results up to commit 2f93595


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


Informational
1. Duplicated SDK logger list ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
tests/test_cli.py hard-codes the sensitive SDK logger names in multiple places instead of asserting
against a single contract, increasing the chance that future changes to the protected-logger
boundary become partially untested. This is a maintainability/coverage risk (especially when adding
new sensitive SDK namespaces).
Code

tests/test_cli.py[R51-52]

+    sdk_loggers = [logging.getLogger(name) for name in ("any_llm", "openai", "httpx", "httpcore")]
+    original_sdk_levels = [logger.level for logger in sdk_loggers]
Relevance

⭐⭐⭐ High

Team previously refactored tests to remove duplicated literals via shared fixtures/constants (PR
#14).

PR-#14

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Production defines a single sensitive-logger tuple used by _configure_logging, but tests duplicate
the logger-name list in multiple places, which can drift as the logging boundary evolves.

weather_briefing/cli.py[159-199]
tests/test_cli.py[45-118]

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

### Issue description
`tests/test_cli.py` duplicates the sensitive SDK logger-name list (e.g., `("any_llm", "openai", "httpx", "httpcore")`) in multiple tests, while production uses `weather_briefing.cli._SENSITIVE_SDK_LOGGERS`. This duplication can drift over time, reducing confidence that changes to the logging boundary are fully covered.

### Issue Context
- Production behavior is controlled by `_SENSITIVE_SDK_LOGGERS` in `weather_briefing/cli.py`.
- Tests currently re-state the same names manually.

### Fix Focus Areas
- tests/test_cli.py[45-118]
- weather_briefing/cli.py[159-199]

### Suggested fix
Choose one of these patterns:
1) Keep an *independent* expected set in tests (to preserve security intent), and add an assertion that production `_SENSITIVE_SDK_LOGGERS` contains at least those required names.
2) Or expose a small public constant (e.g., `weather_briefing.logging_policy.SENSITIVE_SDK_LOGGERS`) and reference it from both production and tests, plus add a separate test asserting it includes the required namespaces.

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


Qodo Logo

@IceCodeNew
IceCodeNew force-pushed the codex/protect-debug-logs branch from 5fcfd0f to 2f93595 Compare July 17, 2026 03:11
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

Comment thread tests/test_cli.py Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2f93595

@IceCodeNew
IceCodeNew marked this pull request as ready for review July 17, 2026 03:24
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Suppress sensitive SDK debug logs by keeping root logger at WARNING

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Restrict DEBUG to the weather_briefing logger; keep root and SDK loggers at WARNING.
• Add regression coverage to ensure app metadata logs while SDK payloads are suppressed.
• Document the privacy boundary and the DEBUG trade-offs for third-party libraries.
Diagram

graph TD
  A["weather_briefing/cli.py"] --> B["_configure_logging(debug)"] --> D["Root handlers WARNING"] --> E["SDK loggers WARNING"]
  B --> C["App logger DEBUG"]
  F["tests/test_cli.py"] --> B
  H["Docs: logging boundary"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add a root Handler Filter to drop sensitive substrings/fields
  • ➕ Can keep selective third-party DEBUG enabled while still blocking known sensitive patterns
  • ➕ Centralized enforcement at the handler boundary regardless of logger name
  • ➖ Brittle: requires anticipating formats/fields across SDK versions
  • ➖ High risk of false negatives (leaks) or noisy false positives (dropped useful logs)
2. Use an allowlist-based DEBUG configuration via dictConfig
  • ➕ Clear, declarative logger-level policy (only known-safe loggers get DEBUG)
  • ➕ Easier to extend safely as new modules are added
  • ➖ More configuration complexity than the current minimal change
  • ➖ May require broader refactor of current logging setup and tests

Recommendation: Current approach (pin root + known sensitive SDK loggers to WARNING, while enabling DEBUG only for weather_briefing) is the safest and simplest defense-in-depth boundary given you cannot trust third-party DEBUG content. If future troubleshooting needs third-party DEBUG, prefer an allowlist-based dictConfig approach over content-based filtering.

Files changed (4) +56 / -11

Bug fix (1) +6 / -3
cli.pyKeep root/SDK loggers at WARNING while allowing app DEBUG +6/-3

Keep root/SDK loggers at WARNING while allowing app DEBUG

• Introduces a list of sensitive SDK logger namespaces and forces them to WARNING. Pins the root logger and all root handlers to WARNING regardless of the 'debug' flag, preventing third-party clients from emitting full request bodies into logs.

weather_briefing/cli.py

Tests (1) +47 / -7
test_cli.pyAdd regression test for suppressing SDK payload logs under DEBUG +47/-7

Add regression test for suppressing SDK payload logs under DEBUG

• Extends logging configuration assertions to include any-llm/OpenAI/httpx/httpcore and root handler levels. Adds a new test that captures logs and verifies app metadata is present while SDK 'payload' messages are suppressed, even if a provider SDK sets its logger to DEBUG.

tests/test_cli.py

Documentation (2) +3 / -1
design.mdClarify DEBUG boundary to exclude root and SDK loggers +1/-1

Clarify DEBUG boundary to exclude root and SDK loggers

• Updates the design documentation to state that DEBUG increases only the 'weather_briefing' logger level. Explicitly calls out keeping root handler and SDK loggers (any-llm/OpenAI/httpx) at WARNING to prevent full request dumps.

docs/design.md

notes.mdDocument privacy boundary and DEBUG trade-offs +2/-0

Document privacy boundary and DEBUG trade-offs

• Adds a note explaining why root and SDK loggers must remain WARNING even when app DEBUG is enabled. Documents the operational trade-off (less third-party debug visibility) and when to reconsider.

docs/notes.md

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2f93595

@IceCodeNew
IceCodeNew force-pushed the codex/protect-debug-logs branch from 2f93595 to 08648fd Compare July 17, 2026 03:29
@IceCodeNew

IceCodeNew commented Jul 17, 2026

Copy link
Copy Markdown
Owner Author

已按最新设计与 Qodo 意见更新,并在 rebase 到 master@beb7c3d 后整理于 2ad6cba

  1. any-llm、OpenAI、HTTPX/httpcore 等第三方 SDK logger 继续固定为 WARNING,避免原始请求日志泄露 token 或 endpoint;
  2. 现有 diagnostics rendered-text enable --for ... 临时开关在 DEBUG 同时启用时,新增应用 adapter 自己掌控的 LLM system prompt、结构化输入和结构化输出诊断;
  3. 不记录 SDK client 配置、认证信息或请求 endpoint;诊断状态读取失败不会影响 LLM 请求;
  4. 每次 LLM 调用只读取一次 SQLite 临时开关,同一结果同时控制请求与响应日志,避免重复 SQL 并保证诊断成对;
  5. logger 安全边界测试由生产 logger 集合派生,并另有独立的必需 logger 集合断言;
  6. README、design 与 notes 已同步;notes 明确记录“开放 SDK 原始 DEBUG”与“不泄露 endpoint/token”的冲突及采用白名单 adapter 日志的原因;
  7. Codecov 暴露的测试 stub 未覆盖行已移除,覆盖率不下降。

完整验证:637 passed,prek run --all-files 全部通过。

@IceCodeNew
IceCodeNew force-pushed the codex/protect-debug-logs branch from 08648fd to 74ca725 Compare July 17, 2026 03:35
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 74ca725

@IceCodeNew
IceCodeNew force-pushed the codex/protect-debug-logs branch from 74ca725 to 0e651b2 Compare July 17, 2026 04:05
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@IceCodeNew
IceCodeNew force-pushed the codex/protect-debug-logs branch from 0e651b2 to 925f640 Compare July 17, 2026 04:09
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

Comment thread weather_briefing/llm.py
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 925f640

@IceCodeNew
IceCodeNew force-pushed the codex/protect-debug-logs branch from 925f640 to 2ad6cba Compare July 17, 2026 04:22
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2ad6cba

@IceCodeNew
IceCodeNew merged commit 2e1d36b into master Jul 17, 2026
20 checks passed
@IceCodeNew
IceCodeNew deleted the codex/protect-debug-logs branch July 17, 2026 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant