Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jun 26, 2025

Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.

Original issue description:

Please use copilot/fix-1 as the base branch for all pull requests related to this refactor. The special refactor branch is created for these changes.

Overview

Refactor the parallel_matrix_formatter codebase. The main target is to make code as simple as it is possible. To achieve this we will centralize all configuration (YAML and ENV), remove all debugging and color logic, standardize suppression, and ensure all runtime and IPC config is handled in a single, immutable config object.

Goals

  • Simplify code, split into smaller classes where possible.
  • Centralize all configuration and ENV handling in a single config loader class.
  • Remove all debug logic: No code should check debug-related ENV or output debug messages.
  • Remove all color logic: DigitalRainRenderer should output only ASCII (no ANSI, Rainbow, or color ENV checks).
  • Standardize suppression: SuppressionLayer should not read ENV directly, but rely on config; only one suppression level should ever be active.
  • Standardize IPC config: All IPC configuration (socket path, mode) should be set once via config and passed to orchestrator/formatters. Remove all file-based server path discovery and IPC path writing except for compatibility fallback.
  • Freeze config: Config should be immutable after load.
  • Document canonical ENV keys and config format.
  • Add comments describing what code does.

Details

1. Config Loader

  • All ENV parsing and YAML config loading must happen in one place (the config loader).
  • Loader sets ENV values for runtime if necessary (e.g., sets IPC path ENV if not already set for children to see).
  • Loader returns a single, frozen config object, with two sections:
    • yaml: All values from YAML/user config.
    • runtime: All values from ENV or runtime detection (suppression, orchestrator/worker role, IPC path/mode, etc).
  • All code must be updated to use config object only, never ENV directly.
  • Document all supported ENV keys (see below).

2. ENV Keys (canonical list)

  • PARALLEL_MATRIX_FORMATTER_CONFIG — config YAML path
  • PARALLEL_MATRIX_FORMATTER_SERVER — IPC path (set by orchestrator, consumed by workers)
  • PARALLEL_MATRIX_FORMATTER_IPC_MODE — (optional) force 'unix' or 'file'
  • PARALLEL_MATRIX_FORMATTER_ORCHESTRATOR — explicit orchestrator mode (optional)
  • PARALLEL_MATRIX_FORMATTER_NO_SUPPRESS — disable suppression
  • PARALLEL_MATRIX_FORMATTER_SUPPRESS — suppression level for workers
  • PARALLEL_SPLIT_TEST_PROCESSES, PARALLEL_WORKERS, TEST_ENV_NUMBER — parallel test context (if used)

3. Suppression Layer

  • Remove all direct ENV access.
  • Accept suppression config as argument or via config object.
  • Only one suppression level is ever active.
  • Levels and usage should be reviewed for clarity and minimalism.
  • Document suppression levels in code.

4. Orchestrator/Formatter/ProcessFormatter

  • Remove all debug logic and ENV checks for debug.
  • Remove all color/ANSI/Rainbow logic.
  • Remove all file-based server path writing/reading (only use ENV for IPC path).
  • Accept config object only for all runtime and visual configuration.

5. DigitalRainRenderer

  • Remove all color logic. Output should be ASCII-only. Ignore all color config keys.

6. IPC

  • All IPC config (paths, mode) must be provided by config object only.
  • Remove all direct ENV/file discovery for IPC paths.
  • File-based IPC can remain as fallback, but only if explicitly configured.

7. Immutability

  • Config object and all derived options are frozen after loading.

8. Documentation

  • At top of config loader, document all supported ENV keys and YAML config sections.
  • Comment what code does everywhere where it is suitable.

After refactor, no ENV access, debug logic, or color logic should remain outside of config loader. All classes must access config only.

Acceptance Criteria

  • Classes are no more than 100 lines. Each class is located in its own file.
  • No ENV checks or debug/color logic outside config loader
  • All config is available on a frozen config object
  • DigitalRainRenderer is ASCII-only
  • SuppressionLayer is config-driven
  • Orchestrator/formatter/process formatter only use config for all runtime and visual options
  • All public methods and config keys are documented
  • All affected code and specs are updated
  • All code has comment explaining what it does.

Impacted Files

  • lib/parallel_matrix_formatter/formatter.rb
  • lib/parallel_matrix_formatter/process_formatter.rb
  • lib/parallel_matrix_formatter/suppression_layer.rb
  • lib/parallel_matrix_formatter/config_loader.rb
  • lib/parallel_matrix_formatter/orchestrator.rb
  • lib/parallel_matrix_formatter/digital_rain_renderer.rb
  • lib/parallel_matrix_formatter/update_strategies.rb
  • lib/parallel_matrix_formatter/ipc.rb

Please ask for clarification if any part of this refactor is ambiguous, as this is a large and foundational change.

Fixes #3.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI requested a review from vovka June 26, 2025 07:16
Copilot finished work on behalf of vovka June 26, 2025 07:16
@vovka vovka closed this Jun 26, 2025
@vovka vovka deleted the copilot/fix-3 branch June 26, 2025 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Centralize config/ENV, remove debug/color logic, and standardize suppression/IPC

2 participants