Skip to content

Consolidate the three progress channels into one #96

Description

@LegalMarc

Goal

Collapse the three overlapping progress mechanisms into one channel. The bridge schema migration deliberately deferred this until its pattern had proven out in the simpler shapes; that condition is now met.

Dependencies

None — immediately eligible. The migration steps it waited on (#67, #89, #90, #91, #92, #93, #94) are all closed.

Scope

Three mechanisms carry progress today and they overlap:

  1. The rich path, a validated ProgressUpdate pydantic dataclass in src/python/marcut/progress.py, which is the only rich shape since the three-argument fallback was removed in Bridge schema step 4a: validate ProgressUpdate and audit the 3-arg callback path #92.
  2. The mass-event path, five pydantic models validated in emit_mass_event and JSON-encoded into the message field, parsed structurally on the Swift side by ingestProgressPayload in DocumentModels.swift.
  3. Direct print of the same JSON to stdout, in the same function.
  • Design one channel that carries both the phase-level updates and the chunk-level events, and state the design on the issue before implementing it.
  • Migrate ProgressTracker and every emit_mass_event call site onto it.
  • Update the Swift consumer to match, keeping the progress bar's observable behaviour identical.
  • Remove whichever of the three mechanisms the new design makes redundant.

Out of scope

  • Changing what progress is reported, how it is weighted, or how the bar animates. This is a transport consolidation, not a behaviour change.
  • The intra-chunk streaming design itself, which is settled in its own spike.

Automation

afk — rung 0, but see the risk note. This is the largest remaining bridge item and the one with the most behavioural surface.

Acceptance criteria

Required verification

  • source .venv/bin/activate && PYTHONPATH=src/python python3 -m pytest -q tests/test_progress.py tests/test_model_enhanced.py tests/test_pipeline.py
  • source .venv/bin/activate && PYTHONPATH=src/python python3 -m pytest -q
  • swift test --package-path src/swift/MarcutApp
  • swift build --package-path src/swift/MarcutApp
  • source .venv/bin/activate && python3 -m ruff check src/python tests
  • swiftformat --lint src/swift/MarcutApp/Sources src/swift/MarcutApp/Tests

Notes

  • Read docs/design/bridge_schema_migration.md §"Migration Plan" step 4 first. It names this explicitly as the follow-up it will not attempt, and says why: it touches ProgressTracker.__init__'s signature-detection logic and is a larger behavioural change than the migration itself.
  • This is the hottest path in the codebase, firing many times per document. Keep the per-event cost at or below what it is today and say so with a measurement.
  • Bridge schema step 4b: closed model set for progress events, and the token_progress gap #93's history is the cautionary tale for this ticket. A parity test that derives one side from source but restates the other is not a pin; it only catches drift in one direction. Whatever replaces SWIFT_HANDLED_MASS_EVENT_TYPES, derive both sides.
  • token_progress is currently emitted by Python and explicitly ignored by Swift via a documented no-op case. Carry that decision forward deliberately rather than dropping the event or silently starting to consume it.
  • If the design turns out to need a call that is genuinely the maintainer's, post the options on the issue and stop rather than guessing.
  • Maintainer decision (posted after the coder's design writeup comment above; do not re-ask, implement on this basis):
    1. Wire format: Option B. One discriminated pydantic union owns the schema everywhere (replacing ProgressUpdate and the 5 mass-event models). The in-process PythonKit path keeps reading live attributes off the PythonObject (no serialization cost) branching on the union's discriminator; the stdout/subprocess path keeps using the JSON string. Do not switch the in-process path to always-JSON (that was Option A, rejected — it would add per-event encode/decode cost to the hot in-process path with no measured need).
    2. Fold in both pre-existing bugs found during design. Unify the CLI-vs-PythonKit-bridge arity-detection divergence (the second, undocumented inspect.signature check in model_enhanced.py around lines 1262-1277 that decides whether to build the inner tracker) so both execution paths emit through the same single call shape. Also bring LlamaCppRedactionPipeline.process_document's raw 3-tuple (model_enhanced.py:1957-1959, currently bypassing emit_mass_event and any pydantic model) into the unified schema as a chunk_start/chunk_end pair with no mass_total/keepalive/token_progress — preserving its current, sparser behavior (no new event types for llama.cpp), just routing it through the same validated shape as the Ollama/CLI paths.
    3. Delete dead code. PythonBridge.swift's runRedactionWithCLI/parseMARCUTProgress (PythonBridge.swift:3841-4176) has zero callers in the app target — delete both as part of this ticket rather than leaving them or filing separately.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    afkAutonomous workflow-loop eligible

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions