Skip to content

GenerationMetricsReporter is a process-wide global: tests are race-free only because one suite touches it #385

Description

@kpmmmurphy

GenerationMetricsReporter (Sources/M1K3Inference/GenerationMetrics.swift) is one process-wide Mutex<handler?> — a deliberate choice (the MLX provider is rebuilt on every brain swap, so a per-provider callback would need re-wiring each time).

It cost a CI failure on an unrelated PR (#383, run 35375840922): two tests in GenerationMetricsTests write the handler, Swift Testing runs a suite's tests in parallel, and install(nil) from one landed between the other's install and its report (box.last → nil). #383 fixes it with @Suite(.serialized).

That fix is sufficient, not structural (review on #383, 17:55Z): .serialized orders only that suite's own tests. It is safe today because a grep shows no other suite calls install/report (the one production report site is behind M1K3_MLX_INTEGRATION=1). The day a second test file touches the reporter, the same race reappears across suites and nothing flags it.

Options, smallest first:

  1. a #if DEBUG scoped-install helper for tests (withHandler { … }) that serialises on its own lock;
  2. a guard test that fails if any test file other than GenerationMetricsTests references GenerationMetricsReporter.install;
  3. an injectable reporter (instance-scoped, handed to the provider at construction) — removes the global, touches the app's launch path and MLXBrainProvider.

Not urgent. Filed so the "we checked, nothing else touches it" assumption is written down somewhere other than a test file's header.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions