Add daily AI review workflow for latest ingest artifacts#29
Merged
Conversation
|
pr-agent-context report: This run includes failing checks on PR #29.
Diagnose and fix the failing checks below, then push all of these changes in a single commit.
# Failing Checks
## FAIL-1
Type: GitHub Actions job
Workflow: ci-test
Job: lint
Run attempt: 1
Conclusion: failure
Current run: yes
URL: https://github.com/DataHackIL/tfht_enforce_idx/actions/runs/23380967323/job/68020238844
Failed steps: Run Ruff format check
Summary:
lint
## FAIL-2
Type: Commit status
Context: pre-commit.ci - pr
Status: failure
URL: https://results.pre-commit.ci/run/github/1159993403/1774100596.ZlmLEPV8S-qgBM9Pqux0RA
Summary:
checks completed with failuresRun metadata: |
|
pr-agent-context report: This run includes a patch coverage gap on PR #29.
Address the patch coverage gaps below, then push all of these changes in a single commit.
# Patch coverage
Patch test coverage is 78.85%; please raise it to 100%. These are the uncovered code lines:
- src/denbust/news_items/daily_review.py: 84, 108, 111, 136, 149, 188, 193, 197, 249, 265, 269, 270, 272, 276, 297, 298, 318, 319, 320, 321, 322, 323, 324, 325, 327, 328, 329, 330, 331, 332, 334, 343, 347Run metadata: |
|
pr-agent-context report: This run includes a failing check on PR #29.
Diagnose and fix the failing checks below, then push all of these changes in a single commit.
# Failing Checks
## FAIL-1
Type: Commit status
Context: pre-commit.ci - pr
Status: failure
URL: https://results.pre-commit.ci/run/github/1159993403/1774101618.Chs7FRgJT-WEqEWJ_VSixQ
Summary:
checks completed with failuresRun metadata: |
|
pr-agent-context report: This run includes a patch coverage gap on PR #29.
Address the patch coverage gaps below, then push all of these changes in a single commit.
# Patch coverage
Patch test coverage is 99.36%; please raise it to 100%. These are the uncovered code lines:
- src/denbust/news_items/daily_review.py: 347Run metadata: |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an AI-assisted “daily review” layer on top of existing news_items/ingest artifacts, intended to automatically inspect the latest successful daily-state-run outputs and open deduplicated GitHub issues when the run looks suspicious.
Changes:
- Introduces
denbust.news_items.daily_reviewmodule to locate latest ingest artifacts, call Anthropic for issue candidates, and create GitHub issues with fingerprint-based dedup markers. - Adds a new
news-items-daily-reviewGitHub Actions workflow triggered afterdaily-state-runcompletes successfully (and via manual dispatch). - Adds unit tests and README documentation for the new workflow and artifact expectations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/denbust/news_items/daily_review.py |
Core implementation: artifact selection, Anthropic prompt/review parsing, GitHub issue creation + dedup logic, CLI entrypoint. |
.github/workflows/news-items-daily-review.yml |
Workflow wiring to run the module after successful daily ingest and create issues using GITHUB_TOKEN. |
tests/unit/test_daily_review.py |
Unit tests covering artifact resolution, JSON extraction, reviewer parsing behavior, issue client behavior, and CLI env handling. |
README.md |
Documents the daily AI review workflow, artifact locations, and secrets/optional config. |
|
pr-agent-context report: This is a refreshed snapshot of the current PR state.
This run includes unresolved review comments and a patch coverage gap on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, address the patch coverage gaps below, and push all of these changes in a single
commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:323
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633108
Root author: copilot-pull-request-reviewer
Comment:
`DENBUST_REVIEW_MODEL` is read with `os.getenv(..., default)`, but if the workflow sets the env var to an empty string (e.g., optional secret not configured), `os.getenv` returns "" and the default model is not used. This would cause the Anthropic call to run with an empty model name and fail. Consider treating empty/whitespace values as missing (e.g., `os.getenv(...) or <default>` / `.strip()` before fallback).
~~~suggestion
raw_model = os.getenv("DENBUST_REVIEW_MODEL", "").strip()
model = raw_model or "claude-sonnet-4-20250514"
~~~
## REVIEW-2
Location: src/denbust/news_items/daily_review.py:108
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633117
Root author: copilot-pull-request-reviewer
Comment:
The markdown-fence stripping logic fails for single-line fenced responses like "~~~json { ... }~~~" or "~~~json\n{...}~~~" where the JSON starts on the same line as the opening fence (or there are only 1–2 lines). In those cases `payload` becomes empty/"~~~" and `json.loads` will crash. Consider a more robust fence removal (e.g., regex that removes the opening fence + optional language token and the trailing fence even when inline).
~~~suggestion
# First, try to strip a complete fenced block (handles single-line and multi-line forms).
# Examples handled:
# ~~~json {"a": 1}~~~
# ~~~json
# {"a": 1}
# ~~~
fenced_match = re.match(r"^~~~(\w+)?\s*\n?(.*)~~~$", payload, re.DOTALL)
if fenced_match:
payload = fenced_match.group(2).strip()
else:
# Fallback to legacy line-based stripping for incomplete or unusual fences.
lines = payload.splitlines()
if len(lines) >= 3 and lines[-1].strip() == "~~~":
payload = "\n".join(lines[1:-1]).strip()
else:
payload = "\n".join(lines[1:]).strip()
~~~
## REVIEW-3
Location: src/denbust/news_items/daily_review.py:185
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633120
Root author: copilot-pull-request-reviewer
Comment:
Only the first content block is inspected, and if it's not a `TextBlock` (or content is empty) `text` remains "" and `extract_json_block` raises `JSONDecodeError`, failing the workflow. Consider concatenating all `TextBlock` parts (or handling other block types) and raising/handling a clearer error (or returning an empty `ReviewResult`) when no text is available.
~~~suggestion
# Concatenate text from all TextBlock content parts.
text_parts: list[str] = []
if response.content:
for block in response.content:
if isinstance(block, TextBlock):
text_parts.append(block.text)
text = "\n".join(text_parts).strip()
if not text:
# No usable text in the response; treat as no issues rather than failing.
return ReviewResult()
try:
payload = extract_json_block(text)
except json.JSONDecodeError:
# Malformed or non-JSON response; treat as no issues rather than failing.
return ReviewResult()
~~~
## REVIEW-4
Location: src/denbust/news_items/daily_review.py:172
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633127
Root author: copilot-pull-request-reviewer
Comment:
The prompt includes the full pretty-printed `debug_log` JSON. Depending on run size this can easily exceed the model context limit or slow the workflow (indentation also increases payload size), causing Anthropic API failures. Consider truncating large sections (e.g., limit article lists), using a compact JSON encoding, and/or sending only the debug summary + selected excerpts needed for issue triage.
## REVIEW-5
Location: src/denbust/news_items/daily_review.py:244
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633129
Root author: copilot-pull-request-reviewer
Comment:
This only fetches the first page of open issues (`per_page=100`) and ignores pagination. If the repo ever has >100 open issues, fingerprints beyond the first page won't be detected and the workflow may open duplicates. Consider paginating via the `Link` header (or using the search API / label filtering) to ensure full coverage.
~~~suggestion
fingerprints: set[str] = set()
url = f"https://api.github.com/repos/{self._repository}/issues"
params: dict[str, Any] | None = {"state": "open", "per_page": 100}
while url:
response = self._client.get(url, params=params)
response.raise_for_status()
for issue in response.json():
if not isinstance(issue, dict) or "pull_request" in issue:
continue
body = str(issue.get("body") or "")
match = re.search(r"<!-- denbust-review:([a-z0-9._-]+) -->", body)
if match:
fingerprints.add(match.group(1))
link_header = response.headers.get("Link", "")
next_url: str | None = None
if link_header:
for part in link_header.split(","):
section = part.strip().split(";")
if len(section) < 2:
continue
url_part = section[0].strip()
rel_part = section[1].strip()
if rel_part == 'rel="next"' and url_part.startswith("<") and url_part.endswith(">"):
next_url = url_part[1:-1]
break
url = next_url
# The "next" URL is already fully qualified and includes query params.
params = None
~~~
## REVIEW-6
Location: .github/workflows/news-items-daily-review.yml:66
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633132
Root author: copilot-pull-request-reviewer
Comment:
Optional secrets are mapped into env vars unconditionally. In GitHub Actions, an unset secret expands to an empty string, which will override the Python default model selection (`os.getenv('DENBUST_REVIEW_MODEL', ...)` will see ""). Consider omitting these env vars when unset, or update the Python code to treat empty strings as missing so the defaults apply.
~~~suggestion
run: |
if [ -n "${{ secrets.DENBUST_REVIEW_MODEL }}" ]; then
export DENBUST_REVIEW_MODEL="${{ secrets.DENBUST_REVIEW_MODEL }}"
fi
if [ -n "${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}" ]; then
export DENBUST_REVIEW_ISSUE_LABELS="${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}"
fi
python -m denbust.news_items.daily_review
~~~
# Patch coverage
Patch test coverage is 99.36%; please raise it to 100%. These are the uncovered code lines:
- src/denbust/news_items/daily_review.py: 347Run metadata: |
|
pr-agent-context report: This is a refreshed snapshot of the current PR state.
This run includes unresolved review comments on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:199
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633127
Root author: copilot-pull-request-reviewer
Comment:
The prompt includes the full pretty-printed `debug_log` JSON. Depending on run size this can easily exceed the model context limit or slow the workflow (indentation also increases payload size), causing Anthropic API failures. Consider truncating large sections (e.g., limit article lists), using a compact JSON encoding, and/or sending only the debug summary + selected excerpts needed for issue triage.
## REVIEW-2
Location: .github/workflows/news-items-daily-review.yml:66
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633132
Root author: copilot-pull-request-reviewer
Comment:
Optional secrets are mapped into env vars unconditionally. In GitHub Actions, an unset secret expands to an empty string, which will override the Python default model selection (`os.getenv('DENBUST_REVIEW_MODEL', ...)` will see ""). Consider omitting these env vars when unset, or update the Python code to treat empty strings as missing so the defaults apply.
~~~suggestion
run: |
if [ -n "${{ secrets.DENBUST_REVIEW_MODEL }}" ]; then
export DENBUST_REVIEW_MODEL="${{ secrets.DENBUST_REVIEW_MODEL }}"
fi
if [ -n "${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}" ]; then
export DENBUST_REVIEW_ISSUE_LABELS="${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}"
fi
python -m denbust.news_items.daily_review
~~~Run metadata: |
|
pr-agent-context report: This run includes unresolved review comments and a failing check on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, fix the failing checks below, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:199
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633127
Root author: copilot-pull-request-reviewer
Comment:
The prompt includes the full pretty-printed `debug_log` JSON. Depending on run size this can easily exceed the model context limit or slow the workflow (indentation also increases payload size), causing Anthropic API failures. Consider truncating large sections (e.g., limit article lists), using a compact JSON encoding, and/or sending only the debug summary + selected excerpts needed for issue triage.
## REVIEW-2
Location: .github/workflows/news-items-daily-review.yml:66
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633132
Root author: copilot-pull-request-reviewer
Comment:
Optional secrets are mapped into env vars unconditionally. In GitHub Actions, an unset secret expands to an empty string, which will override the Python default model selection (`os.getenv('DENBUST_REVIEW_MODEL', ...)` will see ""). Consider omitting these env vars when unset, or update the Python code to treat empty strings as missing so the defaults apply.
~~~suggestion
run: |
if [ -n "${{ secrets.DENBUST_REVIEW_MODEL }}" ]; then
export DENBUST_REVIEW_MODEL="${{ secrets.DENBUST_REVIEW_MODEL }}"
fi
if [ -n "${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}" ]; then
export DENBUST_REVIEW_ISSUE_LABELS="${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}"
fi
python -m denbust.news_items.daily_review
~~~
# Failing Checks
## FAIL-1
Type: Commit status
Context: pre-commit.ci - pr
Status: failure
URL: https://results.pre-commit.ci/run/github/1159993403/1774103152.dWnJeXqESOiZKd-jYTwRgw
Summary:
checks completed with failuresRun metadata: |
|
pr-agent-context report: This is a refreshed snapshot of the current PR state.
This run includes unresolved review comments on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:199
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633127
Root author: copilot-pull-request-reviewer
Comment:
The prompt includes the full pretty-printed `debug_log` JSON. Depending on run size this can easily exceed the model context limit or slow the workflow (indentation also increases payload size), causing Anthropic API failures. Consider truncating large sections (e.g., limit article lists), using a compact JSON encoding, and/or sending only the debug summary + selected excerpts needed for issue triage.
## REVIEW-2
Location: .github/workflows/news-items-daily-review.yml:66
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633132
Root author: copilot-pull-request-reviewer
Comment:
Optional secrets are mapped into env vars unconditionally. In GitHub Actions, an unset secret expands to an empty string, which will override the Python default model selection (`os.getenv('DENBUST_REVIEW_MODEL', ...)` will see ""). Consider omitting these env vars when unset, or update the Python code to treat empty strings as missing so the defaults apply.
~~~suggestion
run: |
if [ -n "${{ secrets.DENBUST_REVIEW_MODEL }}" ]; then
export DENBUST_REVIEW_MODEL="${{ secrets.DENBUST_REVIEW_MODEL }}"
fi
if [ -n "${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}" ]; then
export DENBUST_REVIEW_ISSUE_LABELS="${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}"
fi
python -m denbust.news_items.daily_review
~~~Run metadata: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29 +/- ##
========================================
Coverage 99.83% 99.84%
========================================
Files 37 38 +1
Lines 3072 3270 +198
========================================
+ Hits 3067 3265 +198
Misses 5 5
🚀 New features to boost your workflow:
|
|
pr-agent-context report: This run includes unresolved review comments and a patch coverage gap on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, address the patch coverage gaps below, and push all of these changes in a single
commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:199
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633127
Root author: copilot-pull-request-reviewer
Comment:
The prompt includes the full pretty-printed `debug_log` JSON. Depending on run size this can easily exceed the model context limit or slow the workflow (indentation also increases payload size), causing Anthropic API failures. Consider truncating large sections (e.g., limit article lists), using a compact JSON encoding, and/or sending only the debug summary + selected excerpts needed for issue triage.
## REVIEW-2
Location: .github/workflows/news-items-daily-review.yml:66
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969633132
Root author: copilot-pull-request-reviewer
Comment:
Optional secrets are mapped into env vars unconditionally. In GitHub Actions, an unset secret expands to an empty string, which will override the Python default model selection (`os.getenv('DENBUST_REVIEW_MODEL', ...)` will see ""). Consider omitting these env vars when unset, or update the Python code to treat empty strings as missing so the defaults apply.
~~~suggestion
run: |
if [ -n "${{ secrets.DENBUST_REVIEW_MODEL }}" ]; then
export DENBUST_REVIEW_MODEL="${{ secrets.DENBUST_REVIEW_MODEL }}"
fi
if [ -n "${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}" ]; then
export DENBUST_REVIEW_ISSUE_LABELS="${{ secrets.DENBUST_REVIEW_ISSUE_LABELS }}"
fi
python -m denbust.news_items.daily_review
~~~
# Patch coverage
Patch test coverage is 99.49%; please raise it to 100%. These are the uncovered code lines:
- src/denbust/news_items/daily_review.py: 285Run metadata: |
|
pr-agent-context report: This run includes a failing check on PR #29.
Diagnose and fix the failing checks below, then push all of these changes in a single commit.
# Failing Checks
## FAIL-1
Type: Commit status
Context: pre-commit.ci - pr
Status: failure
URL: https://results.pre-commit.ci/run/github/1159993403/1774116704.zAGMFp42RwOHidyU3kXldg
Summary:
checks completed with failuresRun metadata: |
|
pr-agent-context report: No unresolved review comments, failing checks, or actionable patch coverage gaps were found on PR
#29. Treat this PR as all clear unless new signals appear.Run metadata: |
|
pr-agent-context report: This is a refreshed snapshot of the current PR state.
This run includes unresolved review comments on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:199
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917863
Root author: copilot-pull-request-reviewer
Comment:
`run_snapshot` and `debug_summary` are dumped into the Anthropic prompt without any compaction, while only `debug_log` is compacted. Since `RunSnapshot` can include a potentially large `items` list, this can bloat the prompt, increase cost, and risk exceeding context limits. Consider applying `_compact_for_prompt` (and token-efficient JSON separators) to `run_snapshot`/`debug_summary` as well, or explicitly selecting only the fields needed for review.
## REVIEW-2
Location: src/denbust/news_items/daily_review.py:220
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917866
Root author: copilot-pull-request-reviewer
Comment:
The reviewer currently swallows empty/invalid Anthropic responses and returns `ReviewResult()` without emitting any diagnostics. In the workflow, this makes it hard to distinguish “no issues found” from “review failed / response was malformed”. Consider logging (or printing) a short reason when parsing fails (e.g., empty response, JSON decode failure), ideally without dumping the full prompt.
## REVIEW-3
Location: .github/workflows/news-items-daily-review.yml:37
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917869
Root author: copilot-pull-request-reviewer
Comment:
Because this workflow runs on `workflow_run`, `actions/checkout` will fetch the default ref by default, which can differ from the SHA that produced the artifacts being reviewed. To keep the review code in sync with the triggering `daily-state-run`, consider checking out `ref: ${{ github.event.workflow_run.head_sha }}` for the code repo (and possibly setting an explicit `fetch-depth`).
~~~suggestion
uses: actions/checkout@v6
with:
# For workflow_run, keep code in sync with the triggering daily-state-run.
# For workflow_dispatch, fall back to the SHA of the dispatch event.
ref: ${{ github.event.workflow_run.head_sha || github.sha }}
fetch-depth: 2
~~~
## REVIEW-4
Location: .github/workflows/news-items-daily-review.yml:24
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917872
Root author: copilot-pull-request-reviewer
Comment:
`DATASET_NAME` and `JOB_NAME` are defined in the workflow `env:` but aren’t referenced anywhere in this workflow (only `STATE_JOB_DIR` is used). Consider removing them to avoid configuration drift, or wiring them into `STATE_JOB_DIR` so there’s a single source of truth.
~~~suggestion
STATE_JOB_DIR: state_repo/news_items/ingest
~~~Run metadata: |
|
pr-agent-context report: This is a refreshed snapshot of the current PR state.
This run includes unresolved review comments on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:199
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917863
Root author: copilot-pull-request-reviewer
Comment:
`run_snapshot` and `debug_summary` are dumped into the Anthropic prompt without any compaction, while only `debug_log` is compacted. Since `RunSnapshot` can include a potentially large `items` list, this can bloat the prompt, increase cost, and risk exceeding context limits. Consider applying `_compact_for_prompt` (and token-efficient JSON separators) to `run_snapshot`/`debug_summary` as well, or explicitly selecting only the fields needed for review.
## REVIEW-2
Location: src/denbust/news_items/daily_review.py:220
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917866
Root author: copilot-pull-request-reviewer
Comment:
The reviewer currently swallows empty/invalid Anthropic responses and returns `ReviewResult()` without emitting any diagnostics. In the workflow, this makes it hard to distinguish “no issues found” from “review failed / response was malformed”. Consider logging (or printing) a short reason when parsing fails (e.g., empty response, JSON decode failure), ideally without dumping the full prompt.
## REVIEW-3
Location: .github/workflows/news-items-daily-review.yml:37
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917869
Root author: copilot-pull-request-reviewer
Comment:
Because this workflow runs on `workflow_run`, `actions/checkout` will fetch the default ref by default, which can differ from the SHA that produced the artifacts being reviewed. To keep the review code in sync with the triggering `daily-state-run`, consider checking out `ref: ${{ github.event.workflow_run.head_sha }}` for the code repo (and possibly setting an explicit `fetch-depth`).
~~~suggestion
uses: actions/checkout@v6
with:
# For workflow_run, keep code in sync with the triggering daily-state-run.
# For workflow_dispatch, fall back to the SHA of the dispatch event.
ref: ${{ github.event.workflow_run.head_sha || github.sha }}
fetch-depth: 2
~~~
## REVIEW-4
Location: .github/workflows/news-items-daily-review.yml:24
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917872
Root author: copilot-pull-request-reviewer
Comment:
`DATASET_NAME` and `JOB_NAME` are defined in the workflow `env:` but aren’t referenced anywhere in this workflow (only `STATE_JOB_DIR` is used). Consider removing them to avoid configuration drift, or wiring them into `STATE_JOB_DIR` so there’s a single source of truth.
~~~suggestion
STATE_JOB_DIR: state_repo/news_items/ingest
~~~Run metadata: |
|
pr-agent-context report: This run includes unresolved review comments and a failing check on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, fix the failing checks below, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:205
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917863
Root author: copilot-pull-request-reviewer
Comment:
`run_snapshot` and `debug_summary` are dumped into the Anthropic prompt without any compaction, while only `debug_log` is compacted. Since `RunSnapshot` can include a potentially large `items` list, this can bloat the prompt, increase cost, and risk exceeding context limits. Consider applying `_compact_for_prompt` (and token-efficient JSON separators) to `run_snapshot`/`debug_summary` as well, or explicitly selecting only the fields needed for review.
## REVIEW-2
Location: src/denbust/news_items/daily_review.py:228
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917866
Root author: copilot-pull-request-reviewer
Comment:
The reviewer currently swallows empty/invalid Anthropic responses and returns `ReviewResult()` without emitting any diagnostics. In the workflow, this makes it hard to distinguish “no issues found” from “review failed / response was malformed”. Consider logging (or printing) a short reason when parsing fails (e.g., empty response, JSON decode failure), ideally without dumping the full prompt.
## REVIEW-3
Location: .github/workflows/news-items-daily-review.yml:35
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917869
Root author: copilot-pull-request-reviewer
Comment:
Because this workflow runs on `workflow_run`, `actions/checkout` will fetch the default ref by default, which can differ from the SHA that produced the artifacts being reviewed. To keep the review code in sync with the triggering `daily-state-run`, consider checking out `ref: ${{ github.event.workflow_run.head_sha }}` for the code repo (and possibly setting an explicit `fetch-depth`).
~~~suggestion
uses: actions/checkout@v6
with:
# For workflow_run, keep code in sync with the triggering daily-state-run.
# For workflow_dispatch, fall back to the SHA of the dispatch event.
ref: ${{ github.event.workflow_run.head_sha || github.sha }}
fetch-depth: 2
~~~
# Failing Checks
## FAIL-1
Type: GitHub Actions job
Workflow: ci-test
Job: lint
Run attempt: 1
Conclusion: failure
Current run: yes
URL: https://github.com/DataHackIL/tfht_enforce_idx/actions/runs/23385998782/job/68033044542
Failed steps: Run Ruff format check
Summary:
lintRun metadata: |
for more information, see https://pre-commit.ci
|
pr-agent-context report: This is a refreshed snapshot of the current PR state.
This run includes unresolved review comments on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:205
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917863
Root author: copilot-pull-request-reviewer
Comment:
`run_snapshot` and `debug_summary` are dumped into the Anthropic prompt without any compaction, while only `debug_log` is compacted. Since `RunSnapshot` can include a potentially large `items` list, this can bloat the prompt, increase cost, and risk exceeding context limits. Consider applying `_compact_for_prompt` (and token-efficient JSON separators) to `run_snapshot`/`debug_summary` as well, or explicitly selecting only the fields needed for review.
## REVIEW-2
Location: src/denbust/news_items/daily_review.py:228
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917866
Root author: copilot-pull-request-reviewer
Comment:
The reviewer currently swallows empty/invalid Anthropic responses and returns `ReviewResult()` without emitting any diagnostics. In the workflow, this makes it hard to distinguish “no issues found” from “review failed / response was malformed”. Consider logging (or printing) a short reason when parsing fails (e.g., empty response, JSON decode failure), ideally without dumping the full prompt.
## REVIEW-3
Location: .github/workflows/news-items-daily-review.yml:35
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917869
Root author: copilot-pull-request-reviewer
Comment:
Because this workflow runs on `workflow_run`, `actions/checkout` will fetch the default ref by default, which can differ from the SHA that produced the artifacts being reviewed. To keep the review code in sync with the triggering `daily-state-run`, consider checking out `ref: ${{ github.event.workflow_run.head_sha }}` for the code repo (and possibly setting an explicit `fetch-depth`).
~~~suggestion
uses: actions/checkout@v6
with:
# For workflow_run, keep code in sync with the triggering daily-state-run.
# For workflow_dispatch, fall back to the SHA of the dispatch event.
ref: ${{ github.event.workflow_run.head_sha || github.sha }}
fetch-depth: 2
~~~Run metadata: |
|
pr-agent-context report: This is a refreshed snapshot of the current PR state.
This run includes unresolved review comments on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:205
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917863
Root author: copilot-pull-request-reviewer
Comment:
`run_snapshot` and `debug_summary` are dumped into the Anthropic prompt without any compaction, while only `debug_log` is compacted. Since `RunSnapshot` can include a potentially large `items` list, this can bloat the prompt, increase cost, and risk exceeding context limits. Consider applying `_compact_for_prompt` (and token-efficient JSON separators) to `run_snapshot`/`debug_summary` as well, or explicitly selecting only the fields needed for review.
## REVIEW-2
Location: src/denbust/news_items/daily_review.py:228
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917866
Root author: copilot-pull-request-reviewer
Comment:
The reviewer currently swallows empty/invalid Anthropic responses and returns `ReviewResult()` without emitting any diagnostics. In the workflow, this makes it hard to distinguish “no issues found” from “review failed / response was malformed”. Consider logging (or printing) a short reason when parsing fails (e.g., empty response, JSON decode failure), ideally without dumping the full prompt.
## REVIEW-3
Location: .github/workflows/news-items-daily-review.yml:35
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917869
Root author: copilot-pull-request-reviewer
Comment:
Because this workflow runs on `workflow_run`, `actions/checkout` will fetch the default ref by default, which can differ from the SHA that produced the artifacts being reviewed. To keep the review code in sync with the triggering `daily-state-run`, consider checking out `ref: ${{ github.event.workflow_run.head_sha }}` for the code repo (and possibly setting an explicit `fetch-depth`).
~~~suggestion
uses: actions/checkout@v6
with:
# For workflow_run, keep code in sync with the triggering daily-state-run.
# For workflow_dispatch, fall back to the SHA of the dispatch event.
ref: ${{ github.event.workflow_run.head_sha || github.sha }}
fetch-depth: 2
~~~Run metadata: |
|
pr-agent-context report: This run includes unresolved review comments on PR #29.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.
# Other Review Comments
## REVIEW-1
Location: src/denbust/news_items/daily_review.py:205
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917863
Root author: copilot-pull-request-reviewer
Comment:
`run_snapshot` and `debug_summary` are dumped into the Anthropic prompt without any compaction, while only `debug_log` is compacted. Since `RunSnapshot` can include a potentially large `items` list, this can bloat the prompt, increase cost, and risk exceeding context limits. Consider applying `_compact_for_prompt` (and token-efficient JSON separators) to `run_snapshot`/`debug_summary` as well, or explicitly selecting only the fields needed for review.
## REVIEW-2
Location: src/denbust/news_items/daily_review.py:228
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917866
Root author: copilot-pull-request-reviewer
Comment:
The reviewer currently swallows empty/invalid Anthropic responses and returns `ReviewResult()` without emitting any diagnostics. In the workflow, this makes it hard to distinguish “no issues found” from “review failed / response was malformed”. Consider logging (or printing) a short reason when parsing fails (e.g., empty response, JSON decode failure), ideally without dumping the full prompt.
## REVIEW-3
Location: .github/workflows/news-items-daily-review.yml:35
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/29#discussion_r2969917869
Root author: copilot-pull-request-reviewer
Comment:
Because this workflow runs on `workflow_run`, `actions/checkout` will fetch the default ref by default, which can differ from the SHA that produced the artifacts being reviewed. To keep the review code in sync with the triggering `daily-state-run`, consider checking out `ref: ${{ github.event.workflow_run.head_sha }}` for the code repo (and possibly setting an explicit `fetch-depth`).
~~~suggestion
uses: actions/checkout@v6
with:
# For workflow_run, keep code in sync with the triggering daily-state-run.
# For workflow_dispatch, fall back to the SHA of the dispatch event.
ref: ${{ github.event.workflow_run.head_sha || github.sha }}
fetch-depth: 2
~~~Run metadata: |
|
pr-agent-context report: No unresolved review comments, failing checks, or actionable patch coverage gaps were found on PR
#29. Treat this PR as all clear unless new signals appear.Run metadata: |
|
pr-agent-context report: 🚨 `pr-agent-context` failed while preparing PR context.
PR: #29
Error: CalledProcessError: Command '['git', '-C', '/home/runner/work/tfht_enforce_idx/tfht_enforce_idx/caller-repo', 'diff', '--unified=0', 'a26614b2b56e4e64c0dd3a499851cc6a36f80a54...bf91f3912c7789f85af47d47b29b87d2ec3c8326']' returned non-zero exit status 128.
Run: https://github.com/DataHackIL/tfht_enforce_idx/actions/runs/23387253870
The workflow continued gracefully so this failure does not block CI.
Check the job logs for the full traceback.Run metadata: |
|
pr-agent-context report: 🚨 `pr-agent-context` failed while preparing PR context.
PR: #29
Error: CalledProcessError: Command '['git', '-C', '/home/runner/work/tfht_enforce_idx/tfht_enforce_idx/caller-repo', 'diff', '--unified=0', 'a26614b2b56e4e64c0dd3a499851cc6a36f80a54...bf91f3912c7789f85af47d47b29b87d2ec3c8326']' returned non-zero exit status 128.
Run: https://github.com/DataHackIL/tfht_enforce_idx/actions/runs/23387254533
The workflow continued gracefully so this failure does not block CI.
Check the job logs for the full traceback.Run metadata: |
|
pr-agent-context report: 🚨 `pr-agent-context` failed while preparing PR context.
PR: #29
Error: CalledProcessError: Command '['git', '-C', '/home/runner/work/tfht_enforce_idx/tfht_enforce_idx/caller-repo', 'diff', '--unified=0', 'a26614b2b56e4e64c0dd3a499851cc6a36f80a54...bf91f3912c7789f85af47d47b29b87d2ec3c8326']' returned non-zero exit status 128.
Run: https://github.com/DataHackIL/tfht_enforce_idx/actions/runs/23387286782
The workflow continued gracefully so this failure does not block CI.
Check the job logs for the full traceback.Run metadata: |
|
pr-agent-context report: 🚨 `pr-agent-context` failed while preparing PR context.
PR: #29
Error: CalledProcessError: Command '['git', '-C', '/home/runner/work/tfht_enforce_idx/tfht_enforce_idx/caller-repo', 'diff', '--unified=0', 'a26614b2b56e4e64c0dd3a499851cc6a36f80a54...bf91f3912c7789f85af47d47b29b87d2ec3c8326']' returned non-zero exit status 128.
Run: https://github.com/DataHackIL/tfht_enforce_idx/actions/runs/23388926636
The workflow continued gracefully so this failure does not block CI.
Check the job logs for the full traceback.Run metadata: |
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.
Summary
daily-state-runcompletes successfullynews_items / ingestartifacts from the state repo and ask Anthropic whether engineering issues should be openedDetails
This PR adds a small review module at
src/denbust/news_items/daily_review.pyplus a new workflow,news-items-daily-review.yml.The workflow:
workflow_runafterdaily-state-runcompletes successfullyworkflow_dispatchnews-items-ingestenvironmentnews_items/ingest/runs/news_items/ingest/logs/This keeps the implementation narrow:
<!-- denbust-review:<fingerprint> -->Secrets and environment
Required:
STATE_REPO_PATANTHROPIC_API_KEYOptional:
DENBUST_REVIEW_MODELDENBUST_REVIEW_ISSUE_LABELSThe workflow uses the built-in
GITHUB_TOKENwithissues: writepermissions to create issues in this repository.Validation
PYTHONPATH=src uv run pytest -q tests/unit/test_daily_review.pyuv run ruff check src/denbust/news_items/daily_review.py tests/unit/test_daily_review.pyuv run mypy src/denbust/news_items/daily_review.py