From 3416cc90a580a36b05d6816b32cc6eb81c584876 Mon Sep 17 00:00:00 2001 From: Jason Staack Date: Thu, 12 Mar 2026 22:38:16 -0500 Subject: [PATCH] docs(05-02): complete structured change parser plan Co-Authored-By: Claude Opus 4.6 --- .planning/REQUIREMENTS.md | 8 +- .planning/ROADMAP.md | 4 +- .planning/STATE.md | 22 ++-- .../phases/05-diff-engine/05-02-SUMMARY.md | 112 ++++++++++++++++++ 4 files changed, 130 insertions(+), 16 deletions(-) create mode 100644 .planning/phases/05-diff-engine/05-02-SUMMARY.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 0eda8de..45c249a 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -26,8 +26,8 @@ - [x] **DIFF-01**: Unified diff generated when new snapshot differs from previous - [x] **DIFF-02**: Diffs stored in `router_config_diffs` table linking snapshot pairs -- [ ] **DIFF-03**: Structured change parser extracts component, summary, and raw line as JSON -- [ ] **DIFF-04**: Parsed changes stored in `router_config_changes` table +- [x] **DIFF-03**: Structured change parser extracts component, summary, and raw line as JSON +- [x] **DIFF-04**: Parsed changes stored in `router_config_changes` table ### API @@ -81,8 +81,8 @@ | STOR-05 | Phase 1: Database Schema | Complete | | DIFF-01 | Phase 5: Diff Engine | Complete | | DIFF-02 | Phase 5: Diff Engine | Complete | -| DIFF-03 | Phase 5: Diff Engine | Pending | -| DIFF-04 | Phase 5: Diff Engine | Pending | +| DIFF-03 | Phase 5: Diff Engine | Complete | +| DIFF-04 | Phase 5: Diff Engine | Complete | | API-01 | Phase 6: History API | Pending | | API-02 | Phase 6: History API | Pending | | API-03 | Phase 6: History API | Pending | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 2f2b7a6..baea1e1 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -16,7 +16,7 @@ Decimal phases appear between their surrounding integers in numeric order. - [x] **Phase 2: Poller Config Collection** - SSH export, normalization, and NATS publishing from Go poller (completed 2026-03-13) - [ ] **Phase 3: Snapshot Ingestion** - Backend NATS subscriber stores snapshots with SHA256 deduplication - [x] **Phase 4: Manual Backup Trigger** - API endpoint for on-demand config backup via poller (completed 2026-03-13) -- [ ] **Phase 5: Diff Engine** - Unified diff generation and structured change parsing +- [x] **Phase 5: Diff Engine** - Unified diff generation and structured change parsing (completed 2026-03-13) - [ ] **Phase 6: History API** - REST endpoints for timeline, snapshot view, and diff retrieval with RBAC - [ ] **Phase 7: Config History UI** - Timeline section on device page with change summaries - [ ] **Phase 8: Diff Viewer & Download** - Unified diff display with syntax highlighting and .rsc download @@ -178,7 +178,7 @@ Note: Phase 9 depends only on Phase 3 and Phase 10 depends on Phases 3/4/5, so P | 2. Poller Config Collection | 2/2 | Complete | 2026-03-13 | | 3. Snapshot Ingestion | 0/1 | Not started | - | | 4. Manual Backup Trigger | 1/1 | Complete | 2026-03-13 | -| 5. Diff Engine | 1/2 | In Progress| | +| 5. Diff Engine | 2/2 | Complete | 2026-03-13 | | 6. History API | 0/2 | Not started | - | | 7. Config History UI | 0/1 | Not started | - | | 8. Diff Viewer & Download | 0/2 | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 7d4db0f..86e0c13 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,14 +3,14 @@ gsd_state_version: 1.0 milestone: v9.6 milestone_name: milestone status: completed -stopped_at: Completed 05-01-PLAN.md -last_updated: "2026-03-13T03:34:01.712Z" +stopped_at: Completed 05-02-PLAN.md +last_updated: "2026-03-13T03:37:57.863Z" last_activity: 2026-03-13 -- Completed 05-01 config diff service with TDD progress: total_phases: 10 - completed_phases: 4 + completed_phases: 5 total_plans: 7 - completed_plans: 6 + completed_plans: 7 percent: 86 --- @@ -21,14 +21,14 @@ progress: See: .planning/PROJECT.md (updated 2026-03-12) **Core value:** Operators can see exactly what changed on a router and when, with reliable config snapshots for download -**Current focus:** Phase 5: Diff Engine -- 05-01 COMPLETE +**Current focus:** Phase 5: Diff Engine -- COMPLETE ## Current Position -Phase: 5 of 10 (Diff Engine) -- IN PROGRESS -Plan: 1 of 1 in current phase (05-01 complete) +Phase: 5 of 10 (Diff Engine) -- COMPLETE +Plan: 2 of 2 in current phase (05-02 complete) Status: Phase 5 complete -Last activity: 2026-03-13 -- Completed 05-01 config diff service with TDD +Last activity: 2026-03-13 -- Completed 05-02 structured change parser with TDD Progress: [█████████░] 86% @@ -54,6 +54,7 @@ Progress: [█████████░] 86% *Updated after each plan completion* | Phase 05 P01 | 3min | 2 tasks | 4 files | +| Phase 05 P02 | 2min | 2 tasks | 4 files | ## Accumulated Context @@ -78,6 +79,7 @@ Recent decisions affecting current work: - [Phase 04]: In-process nats-server/v2 for Go unit tests, reused routeros_proxy NATS conn for Python - [Phase 05]: Diff service instantiates own OpenBaoTransitService per-call with close() for clean lifecycle - [Phase 05]: RETURNING id on snapshot INSERT to capture new_snapshot_id without separate query +- [Phase 05]: Change parser is pure function; DB writes in diff service. RETURNING id on diff INSERT for linking. ### Pending Todos @@ -89,6 +91,6 @@ None yet. ## Session Continuity -Last session: 2026-03-13T03:34:01.709Z -Stopped at: Completed 05-01-PLAN.md +Last session: 2026-03-13T03:37:57.861Z +Stopped at: Completed 05-02-PLAN.md Resume file: None diff --git a/.planning/phases/05-diff-engine/05-02-SUMMARY.md b/.planning/phases/05-diff-engine/05-02-SUMMARY.md new file mode 100644 index 0000000..c6908f3 --- /dev/null +++ b/.planning/phases/05-diff-engine/05-02-SUMMARY.md @@ -0,0 +1,112 @@ +--- +phase: 05-diff-engine +plan: 02 +subsystem: api +tags: [parser, routeros, structured-changes, tdd] + +requires: + - phase: 05-diff-engine + plan: 01 + provides: "generate_and_store_diff() and router_config_diffs table" +provides: + - "parse_diff_changes() for extracting structured component changes from unified diffs" + - "router_config_changes rows linked to diff_id for timeline UI" +affects: [07-timeline-api] + +tech-stack: + added: [] + patterns: [tdd-red-green, best-effort-secondary-operation] + +key-files: + created: + - backend/app/services/config_change_parser.py + - backend/tests/test_config_change_parser.py + modified: + - backend/app/services/config_diff_service.py + - backend/tests/test_config_diff_service.py + +key-decisions: + - "Change parser is pure function (no DB/IO) for easy testing; DB writes happen in diff service" + - "RETURNING id added to diff INSERT to capture diff_id for linking changes" + - "Change parser errors are best-effort: diff is always stored, only changes are lost on parser failure" + +patterns-established: + - "RouterOS path to component: strip leading /, replace spaces with / (e.g., /ip firewall filter -> ip/firewall/filter)" + - "Fallback component system/general for diffs without RouterOS path headers" + +requirements-completed: [DIFF-03, DIFF-04] + +duration: 2min +completed: 2026-03-13 +--- + +# Phase 5 Plan 2: Structured Change Parser Summary + +**RouterOS diff change parser extracting component names, human-readable summaries, and raw lines from unified diffs with best-effort DB storage** + +## Performance + +- **Duration:** 2 min +- **Started:** 2026-03-13T03:34:48Z +- **Completed:** 2026-03-13T03:37:14Z +- **Tasks:** 2 +- **Files modified:** 4 + +## Accomplishments +- Pure-function change parser extracts component, summary, raw_line from RouterOS unified diffs +- RouterOS path detection converts section headers to component format (ip/firewall/filter) +- Human-readable summaries: Added/Removed/Modified N rules per component +- Diff service wired to call parser after INSERT and store results in router_config_changes +- Parser failures are best-effort: diff always stored, changes lost only on parser error + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Change parser TDD RED** - `7fddf35` (test) +2. **Task 1: Change parser TDD GREEN** - `b167831` (feat) +3. **Task 2: Wire parser into diff service** - `122b591` (feat) + +_TDD task had separate RED and GREEN commits_ + +## Files Created/Modified +- `backend/app/services/config_change_parser.py` - Pure parser: parse_diff_changes() with path detection, summary generation, raw line capture +- `backend/tests/test_config_change_parser.py` - 6 unit tests covering additions, multi-section, removals, modifications, fallback, raw_line +- `backend/app/services/config_diff_service.py` - Added RETURNING id, parse_diff_changes integration, change INSERT loop +- `backend/tests/test_config_diff_service.py` - Updated existing tests for RETURNING id, added 2 tests for change storage and parser error resilience + +## Decisions Made +- Change parser is a pure function (no DB/IO) for straightforward unit testing; DB writes are the diff service's responsibility +- RETURNING id added to diff INSERT SQL to get diff_id without separate query +- Change parser errors caught by separate try/except so diff is always committed first + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Updated existing diff service tests for RETURNING id and parse_diff_changes integration** +- **Found during:** Task 2 +- **Issue:** Existing tests expected 3 execute calls without scalar_one on INSERT result; new RETURNING id and parse_diff_changes call changed the interaction pattern +- **Fix:** Added scalar_one mock to INSERT result, patched parse_diff_changes to return empty list in existing tests to isolate behavior +- **Files modified:** backend/tests/test_config_diff_service.py +- **Committed in:** 122b591 + +--- + +**Total deviations:** 1 auto-fixed (1 bug) +**Impact on plan:** Necessary test update for API change. No scope creep. + +## Issues Encountered +None + +## User Setup Required +None + +## Next Phase Readiness +- router_config_changes table populated with structured changes for every non-empty diff +- Changes linked to diff_id, device_id, tenant_id for timeline queries +- Ready for timeline API (Phase 7) to query changes per device + +--- +*Phase: 05-diff-engine* +*Completed: 2026-03-13*