Skip to content

Commit f997dc4

Browse files
authored
Merge pull request #235 from EC-DIGIT-CSIRC/issue-169-cached-result-summaries
feat: added result summary implementation to parsers and analysers.
2 parents 1180bcb + 38cefa8 commit f997dc4

157 files changed

Lines changed: 7155 additions & 4620 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/codestyle.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
name: Code Style Check
2+
3+
on:
4+
push:
5+
branches: ["main"]
6+
pull_request:
7+
branches: ["main"]
8+
9+
permissions:
10+
contents: read
11+
12+
jobs:
13+
ruff:
14+
runs-on: ubuntu-latest
15+
steps:
16+
- uses: actions/checkout@v6
17+
18+
- name: Lint
19+
uses: astral-sh/ruff-action@v3
20+
21+
- name: Check formatting
22+
uses: astral-sh/ruff-action@v3
23+
with:
24+
args: "format --check"

.github/workflows/pycodestyle.yml

Lines changed: 0 additions & 35 deletions
This file was deleted.

.github/workflows/unittest.yml

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# This workflow will install Python dependencies, run tests and lint with a single version of Python
1+
# This workflow will install Python dependencies, run tests
22
# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python
33

44
name: Python application
@@ -37,7 +37,7 @@ jobs:
3737
- name: Install Python dependencies
3838
run: |
3939
python -m pip install --upgrade pip
40-
pip install flake8 pytest coverage
40+
pip install pytest coverage
4141
pip install .
4242
- name: Install yara
4343
run: |
@@ -48,21 +48,16 @@ jobs:
4848
fi
4949
mkdir yara
5050
echo -e 'rule match_for_sure_on_ios\n{\n strings:\n $a01 = "/System/Library/CoreServices/powerd.bundle/powerd"\n\n condition:\n any of them\n}\n' > yara/match_for_sure.yar
51-
- name: Lint with flake8
52-
run: |
53-
# stop the build if there are Python syntax errors or undefined names
54-
flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
55-
# exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
56-
flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
51+
5752
- name: Extract sysdiagnose test files ubuntu
5853
run: |
5954
git submodule update --init --recursive
6055
cd tests/testdata
6156
echo "Extracting sysdiagnose test files"
6257
find . -iname '*.tar.gz' -execdir tar xzf {} \; 2> /dev/null
63-
- name: Test with python unittest
58+
- name: Test with python pytest
6459
run: |
65-
python -m coverage run -m unittest -v
60+
python -m coverage run -m pytest tests -v
6661
6762
- name: Generate coverage report
6863
run: |
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"enabled": true,
3+
"name": "Commit Message Helper",
4+
"description": "Help generate conventional commit messages following Git best practices",
5+
"version": "1",
6+
"when": {
7+
"type": "manual",
8+
"button_text": "Generate Commit Message"
9+
},
10+
"then": {
11+
"type": "askAgent",
12+
"prompt": "Help generate a conventional commit message following Git best practices. Please:\n1. Analyze the current git diff to understand what changes were made\n2. Determine the appropriate commit type (feat, fix, docs, style, refactor, test, chore)\n3. Identify the scope if applicable\n4. Generate a concise, descriptive commit message following conventional commit format\n5. Keep the first line under 50 characters and use imperative mood\n6. Suggest the commit message and ask if the user wants to use it\n7. If there are multiple logical changes, suggest splitting into multiple commits\n8. Include body for complex changes if needed"
13+
}
14+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
inclusion: always
3+
---
4+
5+
#[[file:AGENTS.md]]
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
inclusion: fileMatch
3+
fileMatchPattern: "src/sysdiagnose/parsers/**,src/sysdiagnose/analysers/**,tests/test_parsers*,tests/test_analysers*"
4+
---
5+
6+
# Parser & Analyser Development Guidelines
7+
8+
When working on parser or analyser files, follow the detailed guidelines below.
9+
10+
#[[file:docs/developer_guidelines.md]]

AGENTS.md

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# AI Agent Instructions for Sysdiagnose Analysis Framework
2+
3+
This file provides context for AI coding assistants working on this codebase.
4+
Full developer documentation: `docs/developer_guidelines.md`
5+
6+
## Project Structure
7+
8+
```
9+
src/sysdiagnose/
10+
├── parsers/ # Parse raw sysdiagnose files into structured data
11+
├── analysers/ # Process parsed data to produce insights
12+
└── utils/
13+
├── base.py # BaseParserInterface, BaseAnalyserInterface, BaseInterface
14+
└── summary.py # ResultSummary, ResultSummaryExecutionHandler, ResultSummaryFactory
15+
tests/ # Unit tests (one per parser/analyser)
16+
docs/ # Documentation
17+
```
18+
19+
## Architecture
20+
21+
- Parsers extend `BaseParserInterface`, analysers extend `BaseAnalyserInterface`
22+
- Constructor signature: `def __init__(self, config: SysdiagnoseConfig, case: dict)`
23+
- `case` is the full case metadata dict (contains `case_id`, `ios_version`, `model`, etc.)
24+
- `self.case_id` is derived automatically from `case.get("case_id")`
25+
- Only override `execute()` and `get_log_files()` (parsers) — the base class handles I/O, caching, and summary tracking
26+
- Override `_write_result()` only for custom output formats (CSV, GPX, KML)
27+
- Override `_load_output()` only for multi-file parsers
28+
29+
## iOS Version Compatibility
30+
31+
- Declare `ios_version = ">=17.0"` (PEP 440 specifier) on a parser/analyser class to restrict it to specific iOS versions
32+
- Default is `ios_version = "*"` (all versions)
33+
- `_execute_and_write()` automatically skips incompatible versions with `ExecutionStatus.SKIPPED`
34+
- Use `self.is_compatible()` to check programmatically
35+
- The `test_parsers_filestructure` and `test_analysers_filestructure` tests validate that all `ios_version` values are valid PEP 440 specifiers
36+
37+
## Key Rules
38+
39+
- Parsers: `execute()` must guard against missing files: check `get_log_files()` first, `logger.warning()` + return empty if none found
40+
- Parsers: never let `IndexError` propagate from `get_log_files()[0]`
41+
- Never raise exceptions for expected conditions (missing files, empty data, unsupported iOS versions)
42+
- Use `logger` (not `print()`) — warnings/errors are captured by `ResultSummaryExecutionHandler`
43+
- `print()` is enforced by ruff rule `T201` — only CLI entry points are exempt
44+
- Return type must match `format`: json→dict/list, jsonl→list[dict] or Generator[dict], custom→str
45+
- For jsonl, use `Event` dataclass and return `event.to_dict()`
46+
- For large datasets (jsonl only), return a Generator to enable lazy streaming
47+
- Analysers instantiate parsers using `ParserClass(self.config, self.case)` to pass through case metadata
48+
49+
## Tests
50+
51+
- Use `self.subTest(case_id=case_id, ios_version=_case.get('ios_version'))` when iterating over multiple cases so each case is reported independently
52+
- Check compatibility first: `if not p.is_compatible(): self.skipTest(...)`
53+
- Parsers: then check for log files: `if not files: self.fail(...)`
54+
- Use `self.assert_has_required_fields_jsonl(item)` for jsonl validation
55+
- Use `self.assert_result_summary_consistent(instance, result)` to validate summary matches output and **fail on execution errors**
56+
- Use `self.assert_result_summary_consistent(instance, result, allow_errors=True)` for parsers with known upstream issues, but that is not recommended as it will spam the user when using the CLI
57+
- Call `save_result(force=True)` to ensure fresh execution
58+
- Test execution order: parser tests run before analyser tests (configured in `tests/conftest.py`) so analysers benefit from cached parsed data
59+
60+
## Method Lifecycle
61+
62+
```
63+
save_result() → _execute_and_write() → [is_compatible() check] → execute() + _write_result() + ResultSummaryExecutionHandler
64+
```
65+
66+
If incompatible: `_execute_and_write()` returns empty result with `ExecutionStatus.SKIPPED`.
67+
68+
Outliers (custom I/O) use: `execute_with_result_summary()` → execute() + summary (no file write)
69+
70+
The persisted summary file (`<module>.summary.json`) has two purposes:
71+
72+
1. **Trust signal for cached data.** When loading from cache (CLI or API), the summary provides a glimpse of data quality (event count, errors, warnings, timing) without re-executing.
73+
2. **Staleness detection (future).** The `start_time` field enables identifying outdated cached results — relevant for analysers depending on previously parsed data, since CLI logs only capture the current run.
74+
75+
## Do NOT
76+
77+
- Override `save_result()` unless absolutely necessary
78+
- Catch broad `Exception` or `IndexError` to mask missing files
79+
- Return `{"error": "..."}` for missing files — use `logger.warning()` + empty return instead
80+
- Use `print()` for diagnostics — ruff `T201` will reject it
81+
- Hold large datasets in memory when a Generator suffices
82+
- Use `case_id` directly in constructors — always pass the full `case` dict

CONTRIBUTING.md

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# Contributing to Sysdiagnose Analysis Framework
2+
3+
Thank you for your interest in contributing to SAF! This document will help you get started.
4+
5+
## Getting Started
6+
7+
1. Fork the repository
8+
2. Create a virtual environment and install dependencies (see [README.md](README.md#installation))
9+
3. Read the [Developer Guidelines](docs/developer_guidelines.md) before writing code
10+
11+
## Developer Guidelines
12+
13+
All parsers and analysers must follow the conventions documented in [`docs/developer_guidelines.md`](docs/developer_guidelines.md). This includes:
14+
15+
- Architecture and method lifecycle
16+
- How to implement `execute()` and `get_log_files()`
17+
- Error handling (guard against missing files, use `logger.warning()`)
18+
- Output format conventions
19+
- Test structure with `skipTest` for missing data
20+
- Result summary mechanics
21+
22+
## Pull Request Checklist
23+
24+
Before submitting a PR, verify:
25+
26+
- [ ] Class inherits from `BaseParserInterface` or `BaseAnalyserInterface`
27+
- [ ] `description` class attribute is set
28+
- [ ] `format` class attribute matches the functional return type of `execute()`
29+
- [ ] `get_log_files()` returns only files that exist (parsers only)
30+
- [ ] `execute()` guards against empty `get_log_files()` with `logger.warning()` + empty return
31+
- [ ] No `IndexError` can propagate from `get_log_files()[0]` access
32+
- [ ] No `print()` statements (enforced by ruff `T201`)
33+
- [ ] Test file exists under `tests/` with `skipTest` guard for missing data
34+
- [ ] Output validates correctly with test data
35+
- [ ] All existing tests pass
36+
37+
## Running Tests
38+
39+
```bash
40+
python -m unittest discover tests/
41+
```
42+
43+
## Code Style
44+
45+
- Follow existing code patterns in the repository
46+
- Use type hints
47+
- Keep `execute()` focused on parsing logic — let the base class handle I/O and summary tracking
48+
- Prefer `logger.warning()` over raising exceptions for expected conditions (missing files, empty data)
49+
50+
## Reporting Issues
51+
52+
When reporting bugs, please include:
53+
- iOS version of the sysdiagnose archive
54+
- Python version
55+
- Full traceback
56+
- Minimal reproduction steps
57+
58+
## Licence
59+
60+
By contributing, you agree that your contributions will be licensed under the [European Union Public Licence](https://commission.europa.eu/content/european-union-public-licence_en).

Makefile

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,3 @@ PY_FILES := $(shell git ls-files '*.py')
55
test: $(PY_FILES)
66
python -m pytest --verbose tests
77
# or: nosetests -v
8-
9-
10-
lint: $(PY_FILES)
11-
@pycodestyle --exclude=venv --ignore=errors=E221,E225,E251,E501,E266,E302 \
12-
--max-line-length=128 \
13-
$(PY_FILES)

0 commit comments

Comments
 (0)