retrofit: reverse-spec progress-tracking (5 REQs / 9 methods)

lib/Service/ProgressTracker.php

@@ -91,6 +91,8 @@ public function __construct(
      * @param array  $options       Operation options and metadata
      *
      * @return string Unique operation ID
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-1
      */
     public function startOperation(string $operationType, array $options=[]): string
     {
@@ -134,6 +136,8 @@ public function startOperation(string $operationType, array $options=[]): string
      * @param array  $data  Additional phase data
      *
      * @return void
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-2
      */
     public function setPhase(string $phase, array $data=[]): void
     {
@@ -175,6 +179,8 @@ public function setPhase(string $phase, array $data=[]): void
      * @param string $itemType       Type of current item
      *
      * @return void
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-2
      */
     public function updateProgress(int $processedItems=null, string $currentItem=null, string $itemType=null): void
     {
@@ -206,6 +212,8 @@ public function updateProgress(int $processedItems=null, string $currentItem=nul
      * @param string $itemType    Type of current item
      *
      * @return void
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-2
      */
     public function incrementProgress(string $currentItem=null, string $itemType=null): void
     {
@@ -223,6 +231,8 @@ public function incrementProgress(string $currentItem=null, string $itemType=nul
      * @param array  $context Error context
      *
      * @return void
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-3
      */
     public function addError(string $message, array $context=[]): void
     {
@@ -251,6 +261,8 @@ public function addError(string $message, array $context=[]): void
      * @param array  $context Warning context
      *
      * @return void
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-3
      */
     public function addWarning(string $message, array $context=[]): void
     {
@@ -269,6 +281,8 @@ public function addWarning(string $message, array $context=[]): void
      * @param array $statistics Statistics to merge
      *
      * @return void
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-2
      */
     public function updateStatistics(array $statistics): void
     {
@@ -282,6 +296,8 @@ public function updateStatistics(array $statistics): void
      * @param array $finalStatistics Final operation statistics
      *
      * @return void
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-4
      */
     public function completeOperation(array $finalStatistics=[]): void
     {
@@ -315,6 +331,8 @@ public function completeOperation(array $finalStatistics=[]): void
      * @param string $operationId Operation ID to get progress for
      *
      * @return array|null Progress data or null if not found
+     *
+     * @spec openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md#task-5
      */
     public function getProgress(string $operationId=null): ?array
     {

openspec/changes/retrofit-2026-05-24-progress-tracking/design.md

@@ -0,0 +1,26 @@
+# Design — Retrofit Progress Tracking
+
+Retrofit change. Tasks describe retroactive annotation, not new implementation work.
+
+## Context
+
+`ProgressTracker` is a session-backed reporter created circa the ArchiMate import work and reused by sync workflows. It has no existing spec. This change captures observed behavior so the cluster moves from Bucket 2b to Bucket 1 in future scans.
+
+## Decisions
+
+- **Cluster mode (`--cluster progress-tracking`)**, not `--extend`. No existing capability covers this behavior; `method-decomposition` is a refactoring spec, not a feature spec.
+- **5 REQs** consolidate 13 methods. `startOperation` / mutators / error collectors / completion / snapshot access are 5 distinct observable behaviors. Per-method REQs would inflate granularity (e.g. `setPhase` and `updateProgress` describe the same "mutate the active operation" contract from different entry points).
+- **Bug flagged in Notes, not fixed.** `calculateOverallPercentage()` returns 0 because of an empty if-block. The REQ language describes the contract (weighted-phase percentage); the bug is captured in Notes for a follow-up issue. Reverse-spec doctrine: observe, do not fix.
+- **`cleanupOldProgress()` not specified.** Method body is a no-op log statement. Not surfaced as a REQ; track as TODO via follow-up.
+
+## Out of scope
+
+- Refactoring `calculateOverallPercentage()` — open a separate issue.
+- Implementing `cleanupOldProgress()` — open a separate issue.
+- Multi-operation-per-session support — current design is single-slot.
+
+## References
+
+- Umbrella: ConductionNL/softwarecatalog#285
+- Coverage report: openspec/coverage-report.md (2026-05-24)
+- Source: lib/Service/ProgressTracker.php

openspec/changes/retrofit-2026-05-24-progress-tracking/proposal.md

@@ -0,0 +1,27 @@
+# Retrofit — progress-tracking
+
+Describes observed behavior of 13 methods in `ProgressTracker` as 5 new REQs under a new `progress-tracking` capability. Code already exists — this change retroactively specifies it.
+
+## Affected code units
+
+- lib/Service/ProgressTracker.php::startOperation
+- lib/Service/ProgressTracker.php::setPhase
+- lib/Service/ProgressTracker.php::updateProgress
+- lib/Service/ProgressTracker.php::incrementProgress
+- lib/Service/ProgressTracker.php::addError
+- lib/Service/ProgressTracker.php::addWarning
+- lib/Service/ProgressTracker.php::updateStatistics
+- lib/Service/ProgressTracker.php::completeOperation
+- lib/Service/ProgressTracker.php::getProgress
+- lib/Service/ProgressTracker.php::calculateOverallPercentage
+- lib/Service/ProgressTracker.php::calculateEstimatedCompletion
+- lib/Service/ProgressTracker.php::saveProgress
+- lib/Service/ProgressTracker.php::cleanupOldProgress
+
+## Approach
+
+- For each method: describe observed inputs, outputs, pre/postconditions, failure modes from the existing source.
+- Draft REQs that match observed behavior (not aspirational).
+- Notes section surfaces observed-but-suspicious behavior — most notably the `calculateOverallPercentage()` empty-if bug that always returns 0, and the stubbed `cleanupOldProgress()` which only logs.
+
+Source: openspec/coverage-report.md generated 2026-05-24. See [retrofit playbook](../../../../.github/docs/claude/retrofit.md). Umbrella: ConductionNL/softwarecatalog#285.

openspec/changes/retrofit-2026-05-24-progress-tracking/specs/progress-tracking/spec.md

@@ -0,0 +1,98 @@
+---
+status: draft
+retrofit: true
+---
+
+# Progress Tracking Specification
+
+## Purpose
+
+Captures observed behavior of `OCA\SoftwareCatalog\Service\ProgressTracker`, the session-backed progress reporter used by long-running operations (ArchiMate import/export, organisation sync). Tracks a single in-flight operation per session with weighted-phase progress, error/warning collection, and SSE-readable snapshots. Reverse-spec'd from existing code 2026-05-24 to bring the service under spec coverage.
+
+## ADDED Requirements
+
+### REQ-001: The system SHALL start a tracked operation and return a unique operation id
+
+`startOperation(operationType, options)` MUST initialise the in-memory progress state with phase `initializing`, `processed_items = 0`, `start_time = time()`, and `total_items` / `statistics` from `options` when supplied. It MUST generate a unique operation id of the form `{operationType}_{uniqid-with-entropy}` and persist the initial snapshot to the session before returning.
+
+#### Scenario: New operation receives a unique id
+- GIVEN no operation is currently being tracked
+- WHEN `startOperation('import', ['total_items' => 42])` is called
+- THEN the returned id MUST match the pattern `import_<entropy>`
+- AND `getProgress()` MUST return a snapshot whose `phase` equals `'initializing'`
+- AND whose `total_items` equals `42`
+- AND whose `start_time` is set to the current Unix timestamp
+
+#### Scenario: Snapshot is persisted to session immediately
+- GIVEN `startOperation('export')` has been called
+- WHEN the session is inspected for key `progress_<operationId>`
+- THEN the stored value MUST equal the in-memory progress state
+
+### REQ-002: The system SHALL update phase, items, and statistics for the active operation
+
+`setPhase(phase, data)`, `updateProgress(processedItems, currentItem, itemType)`, `incrementProgress(currentItem, itemType)`, and `updateStatistics(statistics)` MUST mutate the in-memory progress state and persist the new snapshot to the session on each call. `setPhase` MUST ignore unknown phase identifiers (log a warning, no state change). `updateStatistics` MUST array-merge the supplied stats into the existing `statistics` map.
+
+#### Scenario: Unknown phase is rejected
+- GIVEN an operation is in phase `initializing`
+- WHEN `setPhase('not-a-real-phase')` is called
+- THEN the progress state's `phase` MUST remain `'initializing'`
+- AND a warning MUST be logged with the unknown phase identifier
+
+#### Scenario: incrementProgress advances by one
+- GIVEN an operation with `processed_items = 5`
+- WHEN `incrementProgress('Element-X', 'element')` is called
+- THEN `processed_items` MUST equal `6`
+- AND `current_item_name` MUST equal `'Element-X'`
+
+#### Scenario: Statistics are merged not replaced
+- GIVEN an operation with `statistics = { elements: 10 }`
+- WHEN `updateStatistics(['relationships' => 5])` is called
+- THEN `statistics` MUST equal `{ elements: 10, relationships: 5 }`
+
+### REQ-003: The system SHALL collect operation errors and warnings without aborting
+
+`addError(message, context)` and `addWarning(message, context)` MUST append a `{ message, context, timestamp }` record to the `errors` or `warnings` array respectively and persist the snapshot. Errors MUST also be logged at error level; warnings are not auto-logged. Neither method MUST throw — callers can continue the operation after recording.
+
+#### Scenario: Error is recorded and logged
+- GIVEN an in-flight operation
+- WHEN `addError('Parse failed', ['file' => 'a.xml'])` is called
+- THEN the progress snapshot's `errors` array MUST contain one entry with `message = 'Parse failed'` and `context.file = 'a.xml'`
+- AND an error-level log entry MUST have been emitted with `operation_id`, `message`, and `context`
+
+### REQ-004: The system SHALL finalise an operation and snapshot its terminal state
+
+`completeOperation(finalStatistics)` MUST set `phase = 'completed'`, `percentage = 100`, `processed_items = total_items`, and `estimated_completion = time()`. If `finalStatistics` is non-empty it MUST be merged into `statistics`. The terminal snapshot MUST be persisted and an info-level log entry emitted with duration, total items, error count, and warning count.
+
+#### Scenario: Completion forces 100% and merges final stats
+- GIVEN an in-flight operation with `percentage = 50` and `statistics = { processed: 10 }`
+- WHEN `completeOperation(['skipped' => 2])` is called
+- THEN `phase` MUST equal `'completed'`
+- AND `percentage` MUST equal `100`
+- AND `statistics` MUST equal `{ processed: 10, skipped: 2 }`
+- AND an info-level log entry MUST include `operation_id` and `duration`
+
+### REQ-005: The system SHALL expose progress snapshots for the active or a referenced operation
+
+`getProgress(operationId)` MUST return the in-memory snapshot when `operationId` is null and the current operation has a non-null id. When `operationId` is supplied and does not match the in-memory operation, it MUST look up the snapshot from session key `progress_{operationId}`. It MUST return `null` when neither match yields a snapshot.
+
+#### Scenario: Active operation snapshot
+- GIVEN an in-flight operation with id `import_abc`
+- WHEN `getProgress()` is called with no argument
+- THEN it MUST return the current progress array
+
+#### Scenario: Cross-session lookup by id
+- GIVEN session key `progress_import_xyz` holds a previously-persisted snapshot
+- WHEN `getProgress('import_xyz')` is called
+- THEN it MUST return the stored snapshot
+
+#### Scenario: Unknown id returns null
+- GIVEN no in-flight operation and no matching session key
+- WHEN `getProgress('does-not-exist')` is called
+- THEN it MUST return `null`
+
+## Notes
+
+- **Bug — `calculateOverallPercentage()` always returns 0.** The current implementation computes `$completedWeight + $currentPhaseProgress` into `$overallProgress` but never assigns the result to `$percentage`. The `if ($totalWeight > 0) { }` block is empty (lines 373-374). `$percentage` is initialised to 0 and stays 0. The REQ above intentionally describes the *contract* (percentage of total work completed) rather than the buggy implementation — see follow-up issue.
+- **`cleanupOldProgress()` is a stub.** It only logs the request; no session pruning happens. Not specified as a REQ — track as TODO.
+- **Single-operation-per-session invariant.** The class stores one operation in `$this->progress`; starting a new one overwrites the previous. The cross-session `getProgress($id)` lookup mitigates this only for snapshots already persisted.
+- **Acceptance Criteria:** ProgressTracker is used by ArchiMate import/export controllers + SettingsController's `streamProgress` SSE endpoint. Behaviour preservation is asserted via integration tests on those callers, not direct unit coverage of ProgressTracker.

openspec/changes/retrofit-2026-05-24-progress-tracking/tasks.md

@@ -0,0 +1,7 @@
+# Tasks
+
+- [x] task-1: progress-tracking#REQ-001 — System SHALL start a tracked operation and return a unique operation id (retroactive annotation)
+- [x] task-2: progress-tracking#REQ-002 — System SHALL update phase, items, and statistics for the active operation (retroactive annotation)
+- [x] task-3: progress-tracking#REQ-003 — System SHALL collect operation errors and warnings without aborting (retroactive annotation)
+- [x] task-4: progress-tracking#REQ-004 — System SHALL finalise an operation and snapshot its terminal state (retroactive annotation)
+- [x] task-5: progress-tracking#REQ-005 — System SHALL expose progress snapshots for the active or a referenced operation (retroactive annotation)