chore: add python qe-on-duty reporter#595
Open
odockal wants to merge 2 commits into
Open
Conversation
Signed-off-by: Ondrej Dockal <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces a configurable QE on-duty Python CLI that collects GitHub PRs, workflow runs, Dependabot alerts, and issues, persists daily snapshots, and generates daily and weekly Markdown reports. ChangesQE On-Duty Reporter
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant Main
participant Collectors
participant GHClient
participant GitHub
participant Reporters
Operator->>Main: run daily or weekly command
Main->>GHClient: initialize and verify authentication
Main->>Collectors: collect configured repositories
Collectors->>GHClient: request PRs, workflows, issues, and alerts
GHClient->>GitHub: execute gh commands and API requests
GitHub-->>GHClient: return JSON data
GHClient-->>Collectors: provide query results
Collectors-->>Main: return normalized records
Main->>Reporters: generate Markdown report
Reporters-->>Main: save daily or weekly report
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (5)
qe-on-duty-reporter/qe_on_duty/collectors/workflow_collector.py (1)
56-57: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAvoid timezone ambiguity in GitHub CLI filters.
The
created_filteris generated as a naive datetime string without a timezone indicator (e.g.,2026-07-15T18:00:00). When passed to the GitHub CLI, it may be interpreted as UTC rather than local time, leading to an incorrect filtering window.Make the datetime timezone-aware and convert it to a UTC string with the
Zsuffix.♻️ Proposed refactor
Add the required imports at the top:
from datetime import datetime, timedelta, timezoneThen update the cutoff calculation and formatting:
def _get_overnight_cutoff(self) -> datetime: """Yesterday at overnight_start_hour.""" - now = datetime.now() + now = datetime.now().astimezone() # Make aware of local timezone yesterday = now - timedelta(days=1) return yesterday.replace(hour=self.overnight_start_hour, minute=0, second=0, microsecond=0) def _collect_from_repo(self, repo: str) -> List[WorkflowData]: cutoff = self._get_overnight_cutoff() - created_filter = f">={cutoff.strftime('%Y-%m-%dT%H:%M:%S')}" + # Convert local cutoff to UTC to pass a deterministic ISO 8601 string to GitHub + created_filter = f">={cutoff.astimezone(timezone.utc).strftime('%Y-%m-%dT%H:%M:%SZ')}"🤖 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 `@qe-on-duty-reporter/qe_on_duty/collectors/workflow_collector.py` around lines 56 - 57, Update the cutoff handling in _get_overnight_cutoff and the created_filter construction to use a timezone-aware datetime, convert it to UTC, and format it with a trailing Z so GitHub CLI receives an unambiguous UTC timestamp.qe-on-duty-reporter/qe_on_duty/gh_client.py (2)
98-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve exception context when raising new exceptions.
When raising a new exception from within an
exceptblock, usefrom e(orfrom Noneif deliberately suppressing the original) to preserve the stack trace. This aids significantly in debugging.♻️ Proposed refactor
- except subprocess.TimeoutExpired: + except subprocess.TimeoutExpired as e: if retry_count < self.max_retries: print(f"⏱️ Command timed out. Retrying {retry_count + 1}/{self.max_retries}...") return self.run_command(args, retry_count + 1) - raise RuntimeError(f"Command timed out after {self.max_retries} retries") + raise RuntimeError(f"Command timed out after {self.max_retries} retries") from e except json.JSONDecodeError as e: - raise RuntimeError(f"Failed to parse gh command output as JSON: {e}") + raise RuntimeError(f"Failed to parse gh command output as JSON: {e}") from e🤖 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 `@qe-on-duty-reporter/qe_on_duty/gh_client.py` around lines 98 - 105, Update the JSONDecodeError handler in run_command to chain the new RuntimeError with the caught exception using explicit exception context, preserving the original parsing traceback while retaining the existing error message.Source: Linters/SAST tools
51-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer unpacking over list concatenation.
Using list unpacking
[*...]is more idiomatic and slightly faster than list concatenation+in Python.♻️ Proposed refactor
result = subprocess.run( - [self.gh_path] + args, + [self.gh_path, *args], capture_output=True,🤖 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 `@qe-on-duty-reporter/qe_on_duty/gh_client.py` around lines 51 - 52, Update the subprocess argument construction in the gh client method containing subprocess.run to use list unpacking for self.gh_path and args instead of list concatenation, while preserving the command argument order.Source: Linters/SAST tools
qe-on-duty-reporter/qe_on_duty/collectors/issue_collector.py (1)
79-81: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove ineffective PR check.
As noted above,
pull_requestis not included in the JSON fields requested bygh_client.py, so this check evaluates toTruefor all items, even if they were PRs.♻️ Proposed refactor
else: - # Return all issues (still filter out PRs) - return [self._parse_issue(issue, repo) for issue in issues if 'pull_request' not in issue] + return [self._parse_issue(issue, repo) for issue in issues]🤖 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 `@qe-on-duty-reporter/qe_on_duty/collectors/issue_collector.py` around lines 79 - 81, Remove the ineffective `'pull_request' not in issue` filter from the fallback return in the issue collection method, and pass all items in `issues` directly to `_parse_issue(issue, repo)` as intended by the surrounding comment.qe-on-duty-reporter/qe_on_duty/collectors/cve_collector.py (1)
40-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid catching blind exceptions during repository collection.
All four collectors iterate over repositories and catch
Exceptionbroadly to prevent a single failure from aborting the run. However, this masks genuine bugs likeNameErrororTypeError. Catch the specific exception expected from the GitHub client instead.
qe-on-duty-reporter/qe_on_duty/collectors/cve_collector.py#L40-L44: changeexcept Exception as e:toexcept RuntimeError as e:qe-on-duty-reporter/qe_on_duty/collectors/issue_collector.py#L43-L44: changeexcept Exception as e:toexcept RuntimeError as e:qe-on-duty-reporter/qe_on_duty/collectors/pr_collector.py#L46-L47: changeexcept Exception as e:toexcept RuntimeError as e:qe-on-duty-reporter/qe_on_duty/collectors/workflow_collector.py#L28-L29: changeexcept Exception as e:toexcept RuntimeError as e:🤖 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 `@qe-on-duty-reporter/qe_on_duty/collectors/cve_collector.py` around lines 40 - 44, Replace the broad Exception handlers with RuntimeError handlers in the repository collection loops of cve_collector.py (lines 40-44), issue_collector.py (lines 43-44), pr_collector.py (lines 46-47), and workflow_collector.py (lines 28-29), preserving the existing warning suppression and logging behavior.Source: Linters/SAST tools
🤖 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 `@qe-on-duty-reporter/main.py`:
- Around line 197-200: The weekly report filename and heading must use one
consistent ISO year/week pair. In qe-on-duty-reporter/main.py lines 197-200,
update the filename logic to unpack iso_year and week_num from end.isocalendar()
and use iso_year; in qe-on-duty-reporter/qe_on_duty/reporters/weekly_reporter.py
lines 60-64, parse the end date and derive the heading from that same ISO
year/week pair instead of the current calendar-week formula.
- Around line 261-266: Update the latest.json replacement logic in the snapshot
creation flow to detect and remove existing entries with os.path.lexists(),
including dangling symlinks, before calling os.symlink(). Preserve the current
target path and success message behavior.
- Around line 146-179: Move the --weekly execution path in the main flow to
immediately after configuration loading and date validation, before GHClient
initialization and repository discovery via get_categorized_repositories and
get_all_repositories. Ensure weekly mode reads existing snapshots and exits
without constructing GHClient or invoking GitHub collection dependencies, while
preserving the normal-mode flow.
- Around line 140-144: Validate and normalize the --date and --time arguments
before constructing timestamp or artifact paths in the report-generation flow.
Ensure report_date cannot contain path separators and report_time is a valid
HHMM value, then build the timestamp from the parsed values; do not append a UTC
“Z” to the local datetime.now() value unless the implementation explicitly
converts it to UTC.
In `@qe-on-duty-reporter/qe_on_duty/collectors/issue_collector.py`:
- Around line 67-78: Update the max-age filtering in the issue collection method
to use timezone-aware UTC values: import and use timezone, compute the cutoff
with datetime.now(timezone.utc), and compare it directly with the parsed
created_at timestamp. Remove the ineffective pull_request membership check while
preserving issue parsing and cutoff filtering.
In `@qe-on-duty-reporter/qe_on_duty/config.py`:
- Around line 22-50: Prevent null configuration values from reaching consumers:
in qe-on-duty-reporter/qe_on_duty/config.py lines 22-50, update _load_config to
initialize safe_load with an empty-dictionary fallback and use `or` fallbacks
for section dictionaries and repository lists; also update
qe-on-duty-reporter/qe_on_duty/config.py lines 101-151 filter getters to use the
appropriate list, dictionary, or scalar fallback. In
qe-on-duty-reporter/qe_on_duty/utils/repository_parser.py lines 40-52, guard
nullable repository, domain, and owners fields with the specified fallbacks. In
qe-on-duty-reporter/qe_on_duty/models/snapshot.py lines 112-116, use empty-list
fallbacks before iterating snapshot arrays.
In `@qe-on-duty-reporter/qe_on_duty/reporters/daily_reporter.py`:
- Line 245: Remove the unnecessary f-string prefixes from the three static
string literals in the daily report construction, including the lines
initialized with “## Report Configuration\n” and the corresponding lines near
the other reported locations. Keep the string contents and report formatting
unchanged.
In `@qe-on-duty-reporter/qe_on_duty/reporters/weekly_reporter.py`:
- Around line 161-164: Update the CVE aggregation loop over self.snapshots and
snapshot.cves to identify alerts by the tuple (repository, alert_number) instead
of repository:cve_id, and retain each alert’s latest severity when processing
snapshots so unique-alert totals do not collapse distinct package or manifest
alerts.
- Around line 129-134: Exclude entries where wf.is_fallback is true from
workflow execution and failure counting in the weekly workflow aggregation at
qe-on-duty-reporter/qe_on_duty/reporters/weekly_reporter.py:129-134, and from
repository workflow activity totals at
qe-on-duty-reporter/qe_on_duty/reporters/weekly_reporter.py:96-100; preserve
counting for non-fallback observations.
- Around line 183-184: Update the snapshot link construction in the weekly
report loop to remove the colon from snapshot.time before composing the daily
report filename, matching the HHMM.md naming used by main.py while preserving
the displayed date and time text.
- Around line 74-89: Update the average calculations in the weekly highlights
generation method to divide avg_prs and total_bugs by the number of snapshots
whose summary is present, matching the existing filtered numerators. Preserve
the zero-snapshot or no-summary behavior by avoiding division by zero and retain
the current formatting.
In `@qe-on-duty-reporter/qe_on_duty/utils/repository_parser.py`:
- Around line 71-81: Update the URL patterns and parsing logic in the repository
parser so repository names may contain dots, while an optional trailing .git
suffix is removed explicitly. Preserve matching for both HTTPS and SSH GitHub
URLs and stop the repository component only at the URL end or the next path
component.
In `@qe-on-duty-reporter/README.md`:
- Around line 50-53: Update the repository navigation command in the README
setup instructions to remove the machine-specific absolute path, using the
repository-relative directory name or a generic placeholder instead.
- Around line 116-130: Update the directory-tree code fences in README.md,
including the analogous fence around the later referenced section, to specify
the text language identifier. Keep the tree contents unchanged so markdownlint
MD040 passes.
In `@qe-on-duty-reporter/requirements.md`:
- Line 5: Update the “Possible actions to take” heading in requirements.md from
level-three to level-two Markdown syntax, preserving the heading text and
correcting the hierarchy for MD001.
- Line 24: In the extensions instruction in requirements.md, correct the
misspelled phrase “go thourgh” to “go through” while leaving the rest of the
user-facing text unchanged.
---
Nitpick comments:
In `@qe-on-duty-reporter/qe_on_duty/collectors/cve_collector.py`:
- Around line 40-44: Replace the broad Exception handlers with RuntimeError
handlers in the repository collection loops of cve_collector.py (lines 40-44),
issue_collector.py (lines 43-44), pr_collector.py (lines 46-47), and
workflow_collector.py (lines 28-29), preserving the existing warning suppression
and logging behavior.
In `@qe-on-duty-reporter/qe_on_duty/collectors/issue_collector.py`:
- Around line 79-81: Remove the ineffective `'pull_request' not in issue` filter
from the fallback return in the issue collection method, and pass all items in
`issues` directly to `_parse_issue(issue, repo)` as intended by the surrounding
comment.
In `@qe-on-duty-reporter/qe_on_duty/collectors/workflow_collector.py`:
- Around line 56-57: Update the cutoff handling in _get_overnight_cutoff and the
created_filter construction to use a timezone-aware datetime, convert it to UTC,
and format it with a trailing Z so GitHub CLI receives an unambiguous UTC
timestamp.
In `@qe-on-duty-reporter/qe_on_duty/gh_client.py`:
- Around line 98-105: Update the JSONDecodeError handler in run_command to chain
the new RuntimeError with the caught exception using explicit exception context,
preserving the original parsing traceback while retaining the existing error
message.
- Around line 51-52: Update the subprocess argument construction in the gh
client method containing subprocess.run to use list unpacking for self.gh_path
and args instead of list concatenation, while preserving the command argument
order.
🪄 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: CHILL
Plan: Pro
Run ID: f490d369-76b5-4361-8912-6b788f346919
📒 Files selected for processing (23)
.gitignoreqe-on-duty-reporter/.gitignoreqe-on-duty-reporter/README.mdqe-on-duty-reporter/config.yamlqe-on-duty-reporter/main.pyqe-on-duty-reporter/qe_on_duty/__init__.pyqe-on-duty-reporter/qe_on_duty/collectors/__init__.pyqe-on-duty-reporter/qe_on_duty/collectors/cve_collector.pyqe-on-duty-reporter/qe_on_duty/collectors/issue_collector.pyqe-on-duty-reporter/qe_on_duty/collectors/pr_collector.pyqe-on-duty-reporter/qe_on_duty/collectors/workflow_collector.pyqe-on-duty-reporter/qe_on_duty/config.pyqe-on-duty-reporter/qe_on_duty/gh_client.pyqe-on-duty-reporter/qe_on_duty/models/__init__.pyqe-on-duty-reporter/qe_on_duty/models/snapshot.pyqe-on-duty-reporter/qe_on_duty/reporters/__init__.pyqe-on-duty-reporter/qe_on_duty/reporters/daily_reporter.pyqe-on-duty-reporter/qe_on_duty/reporters/weekly_reporter.pyqe-on-duty-reporter/qe_on_duty/utils/__init__.pyqe-on-duty-reporter/qe_on_duty/utils/repository_parser.pyqe-on-duty-reporter/requirements.mdqe-on-duty-reporter/requirements.txtqe-on-duty-reporter/tests/__init__.py
Comment on lines
+140
to
+144
| # Parse date and time | ||
| now = datetime.now() | ||
| report_date = args.date if args.date else now.strftime('%Y-%m-%d') | ||
| report_time = args.time if args.time else now.strftime('%H%M') | ||
| timestamp = f"{report_date}T{report_time[:2]}:{report_time[2:]}:00Z" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Parse --date and --time before using them in artifact paths.
These raw strings permit malformed timestamps and path separators in report_date. Also, the appended Z incorrectly claims UTC for a local datetime.now() value.
Proposed fix
now = datetime.now()
report_date = args.date if args.date else now.strftime('%Y-%m-%d')
report_time = args.time if args.time else now.strftime('%H%M')
-timestamp = f"{report_date}T{report_time[:2]}:{report_time[2:]}:00Z"
+try:
+ if len(report_date) != 10 or len(report_time) != 4:
+ raise ValueError
+ report_datetime = datetime.strptime(
+ f"{report_date} {report_time}",
+ "%Y-%m-%d %H%M",
+ )
+except ValueError:
+ parser.error("--date must be YYYY-MM-DD and --time must be HHMM")
+
+timestamp = report_datetime.isoformat(timespec="seconds")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Parse date and time | |
| now = datetime.now() | |
| report_date = args.date if args.date else now.strftime('%Y-%m-%d') | |
| report_time = args.time if args.time else now.strftime('%H%M') | |
| timestamp = f"{report_date}T{report_time[:2]}:{report_time[2:]}:00Z" | |
| # Parse date and time | |
| now = datetime.now() | |
| report_date = args.date if args.date else now.strftime('%Y-%m-%d') | |
| report_time = args.time if args.time else now.strftime('%H%M') | |
| try: | |
| if len(report_date) != 10 or len(report_time) != 4: | |
| raise ValueError | |
| report_datetime = datetime.strptime( | |
| f"{report_date} {report_time}", | |
| "%Y-%m-%d %H%M", | |
| ) | |
| except ValueError: | |
| parser.error("--date must be YYYY-MM-DD and --time must be HHMM") | |
| timestamp = report_datetime.isoformat(timespec="seconds") |
🤖 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 `@qe-on-duty-reporter/main.py` around lines 140 - 144, Validate and normalize
the --date and --time arguments before constructing timestamp or artifact paths
in the report-generation flow. Ensure report_date cannot contain path separators
and report_time is a valid HHMM value, then build the timestamp from the parsed
values; do not append a UTC “Z” to the local datetime.now() value unless the
implementation explicitly converts it to UTC.
Comment on lines
+146
to
+179
| # Initialize GitHub CLI client | ||
| try: | ||
| gh_client = GHClient(config.gh_cli_path, config.max_retries) | ||
| except Exception as e: | ||
| print(f"❌ {e}") | ||
| sys.exit(1) | ||
|
|
||
| # Get current authenticated user | ||
| try: | ||
| current_user = gh_client.get_current_user() | ||
| print(f"👤 Authenticated as: {current_user}") | ||
| except Exception as e: | ||
| print(f"⚠️ Could not detect current user: {e}") | ||
| current_user = "" | ||
|
|
||
| # Get all repositories with source information | ||
| categorized = config.get_categorized_repositories() | ||
| all_repos = config.get_all_repositories() | ||
|
|
||
| print(f"\n📊 Repositories included in this run ({len(all_repos)} total):") | ||
| if categorized['core']: | ||
| print(f"\n Core ({len(categorized['core'])}):") | ||
| for repo in categorized['core']: | ||
| print(f" - {repo}") | ||
| if categorized['domains']: | ||
| print(f"\n Domains.json — owned by {config.domains_user_name} ({len(categorized['domains'])}):") | ||
| for repo in categorized['domains']: | ||
| print(f" - {repo}") | ||
| if categorized['custom']: | ||
| print(f"\n Custom ({len(categorized['custom'])}):") | ||
| for repo in categorized['custom']: | ||
| print(f" - {repo}") | ||
| print() | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle weekly mode before initializing GitHub collection dependencies.
--weekly only reads existing snapshots, but it can currently fail during GHClient initialization or repository discovery before reaching the weekly branch. Move the weekly path immediately after configuration and date validation.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 149-149: Do not catch blind exception: Exception
(BLE001)
[warning] 157-157: Do not catch blind exception: Exception
(BLE001)
🤖 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 `@qe-on-duty-reporter/main.py` around lines 146 - 179, Move the --weekly
execution path in the main flow to immediately after configuration loading and
date validation, before GHClient initialization and repository discovery via
get_categorized_repositories and get_all_repositories. Ensure weekly mode reads
existing snapshots and exits without constructing GHClient or invoking GitHub
collection dependencies, while preserving the normal-mode flow.
Comment on lines
+197
to
+200
| # Determine week number | ||
| week_num = end.isocalendar()[1] | ||
| report_path = os.path.join(config.reports_dir, 'weekly', f"{end.year}-week-{week_num:02d}.md") | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Derive one consistent ISO year/week identifier with isocalendar().
The header formula does not calculate calendar weeks, while the filename combines an ISO week with the potentially different calendar year.
qe-on-duty-reporter/main.py#L197-L200: useiso_year, week_num, _ = end.isocalendar()for the filename.qe-on-duty-reporter/qe_on_duty/reporters/weekly_reporter.py#L60-L64: parse the end date and use the same ISO year/week pair for the heading.
📍 Affects 2 files
qe-on-duty-reporter/main.py#L197-L200(this comment)qe-on-duty-reporter/qe_on_duty/reporters/weekly_reporter.py#L60-L64
🤖 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 `@qe-on-duty-reporter/main.py` around lines 197 - 200, The weekly report
filename and heading must use one consistent ISO year/week pair. In
qe-on-duty-reporter/main.py lines 197-200, update the filename logic to unpack
iso_year and week_num from end.isocalendar() and use iso_year; in
qe-on-duty-reporter/qe_on_duty/reporters/weekly_reporter.py lines 60-64, parse
the end date and derive the heading from that same ISO year/week pair instead of
the current calendar-week formula.
Comment on lines
+261
to
+266
| # Create/update latest.json symlink | ||
| latest_link = os.path.join(day_snapshot_dir, 'latest.json') | ||
| if os.path.exists(latest_link): | ||
| os.remove(latest_link) | ||
| os.symlink(f"{report_time}.json", latest_link) | ||
| print(f"🔗 Latest symlink updated: {latest_link}") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove broken latest.json symlinks before recreating them.
os.path.exists() returns false for a dangling symlink, so os.symlink() then raises FileExistsError after the snapshot has already been saved.
Proposed fix
latest_link = os.path.join(day_snapshot_dir, 'latest.json')
-if os.path.exists(latest_link):
- os.remove(latest_link)
+if os.path.lexists(latest_link):
+ os.unlink(latest_link)
os.symlink(f"{report_time}.json", latest_link)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Create/update latest.json symlink | |
| latest_link = os.path.join(day_snapshot_dir, 'latest.json') | |
| if os.path.exists(latest_link): | |
| os.remove(latest_link) | |
| os.symlink(f"{report_time}.json", latest_link) | |
| print(f"🔗 Latest symlink updated: {latest_link}") | |
| # Create/update latest.json symlink | |
| latest_link = os.path.join(day_snapshot_dir, 'latest.json') | |
| if os.path.lexists(latest_link): | |
| os.unlink(latest_link) | |
| os.symlink(f"{report_time}.json", latest_link) | |
| print(f"🔗 Latest symlink updated: {latest_link}") |
🤖 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 `@qe-on-duty-reporter/main.py` around lines 261 - 266, Update the latest.json
replacement logic in the snapshot creation flow to detect and remove existing
entries with os.path.lexists(), including dangling symlinks, before calling
os.symlink(). Preserve the current target path and success message behavior.
Comment on lines
+67
to
+78
| if self.max_age_days > 0: | ||
| cutoff_date = datetime.now() - timedelta(days=self.max_age_days) | ||
| filtered_issues = [] | ||
| for issue in issues: | ||
| # Filter out pull requests (GitHub API returns PRs as issues) | ||
| if 'pull_request' in issue: | ||
| continue | ||
|
|
||
| created_at = date_parser.parse(issue['createdAt']) | ||
| if created_at.replace(tzinfo=None) >= cutoff_date: | ||
| filtered_issues.append(self._parse_issue(issue, repo)) | ||
| return filtered_issues |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix timezone mismatch and ineffective pull_request check.
There are two issues in this filtering block:
- Timezone mismatch:
datetime.now()returns a naive local datetime, whileissue['createdAt']is parsed as an aware UTC datetime. Using.replace(tzinfo=None)strips the UTC timezone, leading to a comparison between local naive time and UTC naive time. This can cause issues near the age cutoff boundary. - Ineffective PR filter: The
if 'pull_request' in issue:check is ineffective becausepull_requestis not requested in thegh_client.pyJSON fields for issues. (Note:gh issue listgenerally does not return PRs anyway, but if it did, the missing field would cause this check to be bypassed).
Compare aware UTC datetimes directly and remove the ineffective check.
💚 Proposed fix
Update the import at the top of the file to include timezone:
from datetime import datetime, timedelta, timezoneThen update the filtering logic:
# Filter by age if needed
if self.max_age_days > 0:
- cutoff_date = datetime.now() - timedelta(days=self.max_age_days)
+ cutoff_date = datetime.now(timezone.utc) - timedelta(days=self.max_age_days)
filtered_issues = []
for issue in issues:
- # Filter out pull requests (GitHub API returns PRs as issues)
- if 'pull_request' in issue:
- continue
-
created_at = date_parser.parse(issue['createdAt'])
- if created_at.replace(tzinfo=None) >= cutoff_date:
+ if created_at >= cutoff_date:
filtered_issues.append(self._parse_issue(issue, repo))
return filtered_issues📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if self.max_age_days > 0: | |
| cutoff_date = datetime.now() - timedelta(days=self.max_age_days) | |
| filtered_issues = [] | |
| for issue in issues: | |
| # Filter out pull requests (GitHub API returns PRs as issues) | |
| if 'pull_request' in issue: | |
| continue | |
| created_at = date_parser.parse(issue['createdAt']) | |
| if created_at.replace(tzinfo=None) >= cutoff_date: | |
| filtered_issues.append(self._parse_issue(issue, repo)) | |
| return filtered_issues | |
| if self.max_age_days > 0: | |
| cutoff_date = datetime.now(timezone.utc) - timedelta(days=self.max_age_days) | |
| filtered_issues = [] | |
| for issue in issues: | |
| created_at = date_parser.parse(issue['createdAt']) | |
| if created_at >= cutoff_date: | |
| filtered_issues.append(self._parse_issue(issue, repo)) | |
| return filtered_issues |
🤖 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 `@qe-on-duty-reporter/qe_on_duty/collectors/issue_collector.py` around lines 67
- 78, Update the max-age filtering in the issue collection method to use
timezone-aware UTC values: import and use timezone, compute the cutoff with
datetime.now(timezone.utc), and compare it directly with the parsed created_at
timestamp. Remove the ineffective pull_request membership check while preserving
issue parsing and cutoff filtering.
Comment on lines
+71
to
+81
| patterns = [ | ||
| r'https://github\.com/([^/]+/[^/\.]+)', | ||
| r'git@github\.com:([^/]+/[^/\.]+)', | ||
| ] | ||
|
|
||
| for pattern in patterns: | ||
| match = re.match(pattern, url) | ||
| if match: | ||
| return match.group(1) | ||
|
|
||
| return None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix regex to prevent truncating repository names containing dots.
The current regex uses [^/\.]+ to strip the .git extension. However, this incorrectly truncates legitimate repository names that contain internal dots (e.g., owner/my.repo.com or owner/project.io).
Modify the regex to match up to the end of the URL or the next URL component, and explicitly strip the .git suffix if present.
🐛 Proposed fix for robust URL parsing
- patterns = [
- r'https://github\.com/([^/]+/[^/\.]+)',
- r'git@github\.com:([^/]+/[^/\.]+)',
- ]
-
- for pattern in patterns:
- match = re.match(pattern, url)
- if match:
- return match.group(1)
+ patterns = [
+ r'https://github\.com/([^/?#\s]+/[^/?#\s]+)',
+ r'git@github\.com:([^/?#\s]+/[^/?#\s]+)',
+ ]
+
+ for pattern in patterns:
+ match = re.match(pattern, url)
+ if match:
+ repo = match.group(1)
+ if repo.endswith('.git'):
+ repo = repo[:-4]
+ return repo📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| patterns = [ | |
| r'https://github\.com/([^/]+/[^/\.]+)', | |
| r'git@github\.com:([^/]+/[^/\.]+)', | |
| ] | |
| for pattern in patterns: | |
| match = re.match(pattern, url) | |
| if match: | |
| return match.group(1) | |
| return None | |
| patterns = [ | |
| r'https://github\.com/([^/?#\s]+/[^/?#\s]+)', | |
| r'git@github\.com:([^/?#\s]+/[^/?#\s]+)', | |
| ] | |
| for pattern in patterns: | |
| match = re.match(pattern, url) | |
| if match: | |
| repo = match.group(1) | |
| if repo.endswith('.git'): | |
| repo = repo[:-4] | |
| return repo | |
| return None |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 76-76: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.match(pattern, url)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
🤖 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 `@qe-on-duty-reporter/qe_on_duty/utils/repository_parser.py` around lines 71 -
81, Update the URL patterns and parsing logic in the repository parser so
repository names may contain dots, while an optional trailing .git suffix is
removed explicitly. Preserve matching for both HTTPS and SSH GitHub URLs and
stop the repository component only at the URL end or the next path component.
Comment on lines
+50
to
+53
| 1. **Clone or navigate to the repository** | ||
| ```bash | ||
| cd /home/odockal/git/github-apps/qe-on-duty | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the machine-specific installation path.
Line 52 points to a developer-local directory and will not work for other users. Use a repository-relative command such as cd qe-on-duty-reporter or a generic placeholder.
🤖 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 `@qe-on-duty-reporter/README.md` around lines 50 - 53, Update the repository
navigation command in the README setup instructions to remove the
machine-specific absolute path, using the repository-relative directory name or
a generic placeholder instead.
Comment on lines
+116
to
+130
| ``` | ||
| output/ | ||
| ├── snapshots/ | ||
| │ └── 2026-04-21/ | ||
| │ ├── 0930.json # 9:30 AM run | ||
| │ ├── 1430.json # 2:30 PM run | ||
| │ └── latest.json # Symlink to 1430.json | ||
| └── reports/ | ||
| ├── daily/ | ||
| │ └── 2026-04-21/ | ||
| │ ├── 0930.md | ||
| │ └── 1430.md | ||
| └── weekly/ | ||
| └── 2026-week-16.md | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to the directory-tree code fences.
These fences omit a language identifier, triggering markdownlint MD040. Mark them as text (or another appropriate language) so documentation linting passes.
Also applies to: 204-224
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 116-116: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@qe-on-duty-reporter/README.md` around lines 116 - 130, Update the
directory-tree code fences in README.md, including the analogous fence around
the later referenced section, to specify the text language identifier. Keep the
tree contents unchanged so markdownlint MD040 passes.
Source: Linters/SAST tools
|
|
||
| Why? Having a rotating schedule for these responsibilities ensures a balanced distribution of tasks among the QE team, promoting skill development, team collaboration and team member exposure to a broader engineering team. | ||
|
|
||
| ### Possible actions to take |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the heading hierarchy.
Change ### Possible actions to take to ## Possible actions to take; the current heading skips the H2 level and triggers markdownlint MD001.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 5-5: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 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 `@qe-on-duty-reporter/requirements.md` at line 5, Update the “Possible actions
to take” heading in requirements.md from level-three to level-two Markdown
syntax, preserving the heading text and correcting the hierarchy for MD001.
Source: Linters/SAST tools
| * https://github.com/podman-desktop/e2e | ||
|
|
||
| Gathering the test results from the extensions GH Actions CI: | ||
| * go thourgh the list of extensions on domains.json file: https://github.com/containers/podman-desktop-internal/blob/main/domains.json |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the spelling error.
Change go thourgh to go through in this user-facing instruction.
🧰 Tools
🪛 LanguageTool
[grammar] ~24-~24: Ensure spelling is correct
Context: ...from the extensions GH Actions CI: * go thourgh the list of extensions on domains.json ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 `@qe-on-duty-reporter/requirements.md` at line 24, In the extensions
instruction in requirements.md, correct the misspelled phrase “go thourgh” to
“go through” while leaving the rest of the user-facing text unchanged.
Source: Linters/SAST tools
odockal
marked this pull request as draft
July 16, 2026 12:33
Signed-off-by: Ondrej Dockal <[email protected]>
odockal
force-pushed
the
qe-on-duty-report
branch
from
July 17, 2026 17:15
7aa0e0d to
134b723
Compare
odockal
marked this pull request as ready for review
July 17, 2026 18:13
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
1 participant
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.
A python project to collect various information from CI and system tracking system. Outcome is a summary/report in
outputfolder.Follow the readme.
In short, checkout, install deps, adjust config.yaml file, authenticate using gh and run
python main.py.