elastic / elastic/ai-github-actions
[refactor-opportunist] Decouple log error extraction from run-id-in-path assumptions
- Dominant language
- Python
- Stars
- 11
- Forks
- 16
- Avg merge
- 22h 9m
- Merged PRs (30d)
- 31
Description
## 🏗️ Refactor Proposal
**Summary:** Make `manifest.json` the explicit metadata contract between log download and log error extraction so run metadata attachment no longer depends on log file paths containing `run_id`.
## Problem
`extract-log-errors.py` currently has a hidden coupling to directory layout:
- `scripts/extract-log-errors.py:172-184` (`attach_run_metadata`) falls back to scanning path segments and matching a segment to `run_id`.
- This assumes logs are always stored in a `...//...` path shape.
- Any producer that uses a different path convention can still provide a valid manifest but lose run metadata attachment.
At the same time, the downloader already knows file→run mapping when it writes the manifest:
- `scripts/fetch-workflow-logs.py:183-199` now emits explicit per-log records.
This is a structural boundary issue: metadata ownership is split between manifest content and path convention instead of living in one explicit contract.
## Proposed Approach
Standardize on explicit per-log manifest records:
- Producer (`fetch-workflow-logs.py`) emits `logs[]` entries with `path`, `run_id`, `conclusion`, `created_at`, `html_url`.
- Consumer (`extract-log-errors.py`) reads `logs[]` first and attaches run metadata by exact file path.
- Keep legacy `log_files` + path-segment fallback during migration for backward compatibility.
## Proof of Concept
I partially implemented this refactor on one representative slice to verify viability:
**Files changed:**
- `scripts/fetch-workflow-logs.py`
- `scripts/extract-log-errors.py`
- `tests/test_fetch_workflow_logs.py`
- `tests/test_extract_log_errors.py`
**Before → After:**
- **Before:** consumer inferred run metadata from file path segments (`Path(filepath).parts`) and `run_id` string matching.
- **After:**
- producer writes explicit `logs` metadata (`scripts/fetch-workflow-logs.py:189-198`)
- consumer builds `run_meta_by_file` from manifest and attaches metadata via direct path lookup first (`scripts/extract-log-errors.py:150-179`)
- legacy fallback remains for existing manifests (`scripts/extract-log-errors.py:180-184`).
**Verification:**
- `UV_CACHE_DIR=/tmp/gh-aw/agent/uv-cache uv run --with pytest pytest -q tests/test_fetch_workflow_logs.py tests/test_extract_log_errors.py`
- Result: `10 passed`
- Added targeted regression coverage:
- `tests/test_fetch_workflow_logs.py:124-169` validates explicit `logs[]` manifest emission
- `tests/test_extract_log_errors.py:143-178` validates metadata attachment works even when path does not contain `run_id`
## Incremental Rollout Plan
This refactor can be completed incrementally:
1. **Done (proof slice):** add `logs[]` contract and consumer support with backward compatibility.
2. **Next slice:** update internal workflow manifest producers that currently write only `log_files` (e.g. `agent-deep-dive.md`, `agent-efficiency.md`) to emit `logs[]` directly.
3. **Final slice:** after producers are migrated, remove path-segment inference fallback and treat missing per-log metadata as an explicit manifest quality error.
## Risks and Mitigations
- **Risk:** Mixed manifest versions during rollout.
**Mitigation:** Keep legacy `log_files` and run-id path fallback until all producers migrate.
- **Risk:** Slightly larger manifest payloads due to duplicated run fields per log entry.
**Mitigation:** Keep this transitional; if needed later, compact to `logs[{path, run_id}]` plus run table.
## Evidence
- Hidden coupling point: `scripts/extract-log-errors.py:172-184`
- New explicit contract emission: `scripts/fetch-workflow-logs.py:183-199`
- New direct metadata mapping path: `scripts/extract-log-errors.py:135-169`
- Proof tests:
- `tests/test_fetch_workflow_logs.py:124-169`
- `tests/test_extract_log_errors.py:143-178`
---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Trigger Refactor Opportunist](https://github.com/elastic/ai-github-actions/actions/runs/30825687888)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
Contributor guide
Research direction
Start with scripts/fetch-workflow-logs.py and scripts/extract-log-errors.py, then inspect the manifest producers named in agent-deep-dive.md and agent-efficiency.md. Run the two targeted pytest files to understand the existing contract and compatibility coverage. Done means migrated producers emit logs[] records while legacy manifests still work, with tests covering explicit metadata attachment and the rollout changes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github-actions, python
- Domain
- ci-cd, tooling
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100