Skip to content

chore: sync workflow templates - #552

Closed
stranske wants to merge 1 commit into
mainfrom
sync/workflows-4bf5dad971e1
Closed

chore: sync workflow templates#552
stranske wants to merge 1 commit into
mainfrom
sync/workflows-4bf5dad971e1

Conversation

@stranske

@stranske stranske commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Sync Summary

Files Updated

  • aggregate_agent_metrics.py: Aggregates downloaded weekly agent metrics - required by agents-weekly-metrics.yml
  • WORKFLOW_USER_GUIDE.md: Workflow user guide - explains the CI/agent system for repo consumers

Files Skipped

  • pr-00-gate.yml: File exists and sync_mode is create_only
  • ci.yml: File exists and sync_mode is create_only
  • renovate.json: File exists and sync_mode is create_only
  • cross-repo-smoke.yml: File exists and sync_mode is create_only
  • llm_slots.json: None

Review Checklist

  • CI passes with updated workflows
  • No repo-specific customizations were overwritten

Source: stranske/Workflows
Source SHA: 6456318693452d93e6e4be49edb4f82ba62e883a
Template hash: 4bf5dad971e1
Sync branch: sync/workflows-4bf5dad971e1
Consumer repo: stranske/Pension-Data
Manifest: .github/sync-manifest.yml

Summary by CodeRabbit

  • Documentation

    • Clarified workflow guide regarding approval requirements for automated dependency management operations and how manual label application differs in enforcement.
  • Refactor

    • Improved metrics aggregation reliability with enhanced error detection, better data validation handling, and fallback parsing support for legacy data formats.

Automated sync from stranske/Workflows
Template hash: 4bf5dad971e1

Changes synced from sync-manifest.yml
@stranske stranske added sync Automated sync from Workflows automated Automated sync from Workflows labels Jun 16, 2026
@stranske
stranske temporarily deployed to agent-standard June 16, 2026 10:06 — with GitHub Actions Inactive
@stranske
stranske temporarily deployed to agent-standard June 16, 2026 10:06 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Two unrelated changes: aggregate_agent_metrics.py removes its import of read_ndjson_file from src.ndjson_parser and defines an equivalent inline implementation with line-by-line NDJSON parsing, per-line error recording, and a legacy JSON fallback. WORKFLOW_USER_GUIDE.md moves the agents:allow-change label from the user-applied triggers table to the auto-applied status table with a clarified description.

Changes

Inline NDJSON reader in aggregate_agent_metrics.py

Layer / File(s) Summary
Remove external import and add inline read_ndjson_file
scripts/aggregate_agent_metrics.py
Drops the src.ndjson_parser import and defines read_ndjson_file(path: Path) -> tuple[list[dict[str, Any]], list[str]] that parses NDJSON line-by-line into dict entries with per-line JSON/type error accumulation, and falls back to single-object or list-of-objects JSON parsing from a buffer when no entries are parsed.

Workflow guide label reclassification

Layer / File(s) Summary
Relocate and clarify agents:allow-change label entry
WORKFLOW_USER_GUIDE.md
Removes the agents:allow-change row from the "User-Applied Labels (Triggers)" table and adds it to the "Auto-Applied Labels (Status)" table with an explicit note that the CODEOWNER bypass applies only to automated Dependabot/Renovate PRs and that manual application does not bypass guard enforcement.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: sync workflow templates' accurately describes the main change - this is an automated workflow template synchronization pull request that updates workflow-related files from a source repository.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/workflows-4bf5dad971e1

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Workflow state fingerprint for Keepalive Loop Reporter. Do not edit.

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Workflow state fingerprint for Agents Gate Followups. Do not edit.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@scripts/aggregate_agent_metrics.py`:
- Around line 318-345: The code within the `with handle:` block fails to catch
read/decode errors that can occur during the `for line_number, line in
enumerate(handle, start=1):` iteration. Wrap the entire file iteration loop and
all its logic in a try-except block that catches both `UnicodeDecodeError` and
`OSError`. When either exception occurs during iteration, append an appropriate
error message to the `errors` list (similar to how JSON parsing errors are
appended) that includes the path and the exception details, allowing aggregation
to continue and report the malformed artifact rather than crashing.
- Around line 323-363: The legacy JSON fallback mechanism is being disabled
prematurely when a valid JSON object is encountered on a single line. When the
code finds a valid dict and appends it to entries, it clears
raw_lines_for_fallback, which prevents the full-file fallback parse from working
correctly for pretty-printed JSON arrays where individual elements are valid
JSON objects. Remove the line that clears raw_lines_for_fallback when
isinstance(parsed, dict) is true (currently on line 342), so the fallback buffer
continues accumulating lines. This ensures that when parsing errors occur, the
complete buffered content is still available for the full-file fallback parse
attempt at line 350, allowing pretty-printed legacy JSON files to be parsed
correctly instead of returning partial data with errors.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a694de71-f43d-4dbc-8b7d-7bc993aec9b6

📥 Commits

Reviewing files that changed from the base of the PR and between 74da5f3 and ea2e98f.

📒 Files selected for processing (2)
  • WORKFLOW_USER_GUIDE.md
  • scripts/aggregate_agent_metrics.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use conventional commit message format with type prefix: type/description where types include fix, feat, chore, docs. For example: fix: resolve mypy type error or feat: add new authentication method.

Files:

  • scripts/aggregate_agent_metrics.py
🔇 Additional comments (1)
WORKFLOW_USER_GUIDE.md (1)

168-168: LGTM!

Comment on lines +318 to +345
with handle:
for line_number, line in enumerate(handle, start=1):
raw = line.strip()
if not raw:
continue
if not entries and not raw_fallback_truncated:
raw_bytes = len(raw.encode("utf-8")) + 1
fallback_within_limit = (
len(raw_lines_for_fallback) < _MAX_LEGACY_JSON_FALLBACK_LINES
and raw_fallback_bytes + raw_bytes <= _MAX_LEGACY_JSON_FALLBACK_BYTES
)
if fallback_within_limit:
raw_fallback_bytes += raw_bytes
raw_lines_for_fallback.append(raw)
else:
raw_fallback_truncated = True
raw_lines_for_fallback = []
try:
parsed = json.loads(raw)
except json.JSONDecodeError as exc:
errors.append(f"{path}:{line_number}: invalid JSON ({exc.msg})")
continue
if isinstance(parsed, dict):
entries.append(parsed)
raw_lines_for_fallback = []
else:
errors.append(f"{path}:{line_number}: expected object, got {type(parsed).__name__}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Catch read/decode failures during iteration.

path.open() can succeed while the later for line in handle raises UnicodeDecodeError or OSError. In the weekly metrics workflow, one malformed artifact would crash aggregation instead of being reported through parse-error details.

🛡️ Proposed fix
-    with handle:
-        for line_number, line in enumerate(handle, start=1):
-            raw = line.strip()
-            if not raw:
-                continue
-            if not entries and not raw_fallback_truncated:
-                raw_bytes = len(raw.encode("utf-8")) + 1
-                fallback_within_limit = (
-                    len(raw_lines_for_fallback) < _MAX_LEGACY_JSON_FALLBACK_LINES
-                    and raw_fallback_bytes + raw_bytes <= _MAX_LEGACY_JSON_FALLBACK_BYTES
-                )
-                if fallback_within_limit:
-                    raw_fallback_bytes += raw_bytes
-                    raw_lines_for_fallback.append(raw)
-                else:
-                    raw_fallback_truncated = True
-                    raw_lines_for_fallback = []
-            try:
-                parsed = json.loads(raw)
-            except json.JSONDecodeError as exc:
-                errors.append(f"{path}:{line_number}: invalid JSON ({exc.msg})")
-                continue
-            if isinstance(parsed, dict):
-                entries.append(parsed)
-                raw_lines_for_fallback = []
-            else:
-                errors.append(f"{path}:{line_number}: expected object, got {type(parsed).__name__}")
+    try:
+        with handle:
+            for line_number, line in enumerate(handle, start=1):
+                raw = line.strip()
+                if not raw:
+                    continue
+                # existing per-line parsing logic...
+    except (OSError, UnicodeDecodeError) as exc:
+        return entries, errors + [f"{path}: {exc}"]
🤖 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 `@scripts/aggregate_agent_metrics.py` around lines 318 - 345, The code within
the `with handle:` block fails to catch read/decode errors that can occur during
the `for line_number, line in enumerate(handle, start=1):` iteration. Wrap the
entire file iteration loop and all its logic in a try-except block that catches
both `UnicodeDecodeError` and `OSError`. When either exception occurs during
iteration, append an appropriate error message to the `errors` list (similar to
how JSON parsing errors are appended) that includes the path and the exception
details, allowing aggregation to continue and report the malformed artifact
rather than crashing.

Comment on lines +323 to +363
if not entries and not raw_fallback_truncated:
raw_bytes = len(raw.encode("utf-8")) + 1
fallback_within_limit = (
len(raw_lines_for_fallback) < _MAX_LEGACY_JSON_FALLBACK_LINES
and raw_fallback_bytes + raw_bytes <= _MAX_LEGACY_JSON_FALLBACK_BYTES
)
if fallback_within_limit:
raw_fallback_bytes += raw_bytes
raw_lines_for_fallback.append(raw)
else:
raw_fallback_truncated = True
raw_lines_for_fallback = []
try:
parsed = json.loads(raw)
except json.JSONDecodeError as exc:
errors.append(f"{path}:{line_number}: invalid JSON ({exc.msg})")
continue
if isinstance(parsed, dict):
entries.append(parsed)
raw_lines_for_fallback = []
else:
errors.append(f"{path}:{line_number}: expected object, got {type(parsed).__name__}")

if entries or not errors:
return entries, errors

raw_text = "\n".join(raw_lines_for_fallback)
if raw_fallback_truncated:
errors.append(f"{path}: legacy-json-fallback-buffer-limit")
return entries, errors

try:
parsed_file = json.loads(raw_text)
except json.JSONDecodeError:
return entries, errors

if isinstance(parsed_file, dict):
return [parsed_file], []
if isinstance(parsed_file, list) and all(isinstance(item, dict) for item in parsed_file):
return list(parsed_file), []
return entries, errors

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t let an interior object disable the legacy JSON fallback.

A valid pretty-printed legacy JSON array can contain a line that is itself a JSON object, especially the last element. That makes entries non-empty, clears the fallback buffer, and Line 346 returns partial data with parse errors instead of parsing the whole legacy JSON file. Keep the bounded full-file fallback candidate until parsing is complete, and try it whenever line parsing produced errors.

🐛 Proposed fix
-            if not entries and not raw_fallback_truncated:
+            if not raw_fallback_truncated:
                 raw_bytes = len(raw.encode("utf-8")) + 1
                 fallback_within_limit = (
                     len(raw_lines_for_fallback) < _MAX_LEGACY_JSON_FALLBACK_LINES
                     and raw_fallback_bytes + raw_bytes <= _MAX_LEGACY_JSON_FALLBACK_BYTES
                 )
@@
             if isinstance(parsed, dict):
                 entries.append(parsed)
-                raw_lines_for_fallback = []
             else:
                 errors.append(f"{path}:{line_number}: expected object, got {type(parsed).__name__}")
 
-    if entries or not errors:
+    if not errors:
         return entries, errors
 
-    raw_text = "\n".join(raw_lines_for_fallback)
     if raw_fallback_truncated:
-        errors.append(f"{path}: legacy-json-fallback-buffer-limit")
+        if not entries:
+            errors.append(f"{path}: legacy-json-fallback-buffer-limit")
         return entries, errors
 
+    raw_text = "\n".join(raw_lines_for_fallback)
     try:
         parsed_file = json.loads(raw_text)
     except json.JSONDecodeError:
         return entries, errors
🤖 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 `@scripts/aggregate_agent_metrics.py` around lines 323 - 363, The legacy JSON
fallback mechanism is being disabled prematurely when a valid JSON object is
encountered on a single line. When the code finds a valid dict and appends it to
entries, it clears raw_lines_for_fallback, which prevents the full-file fallback
parse from working correctly for pretty-printed JSON arrays where individual
elements are valid JSON objects. Remove the line that clears
raw_lines_for_fallback when isinstance(parsed, dict) is true (currently on line
342), so the fallback buffer continues accumulating lines. This ensures that
when parsing errors occur, the complete buffered content is still available for
the full-file fallback parse attempt at line 350, allowing pretty-printed legacy
JSON files to be parsed correctly instead of returning partial data with errors.

@stranske

Copy link
Copy Markdown
Owner Author

Superseded by newer workflow template sync PR #553 from the latest Workflows source wave.

@stranske stranske closed this Jun 16, 2026
@stranske-keepalive
stranske-keepalive Bot deleted the sync/workflows-4bf5dad971e1 branch June 17, 2026 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated Automated sync from Workflows sync Automated sync from Workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant