elastic / elastic/ai-github-actions

[refactor-opportunist] Decouple log error extraction from run-id-in-path assumptions

Open
#1,750 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.