Skip to content

Split database_options by ingestion vs presentation, retiring the local/global scope distinction #88

Description

@AlexisJanin

Context

Surfaced while grilling architecture ticket 04 (lift PlotGroup assembly out of wrapper.main). Ticket 04 itself resolves the code-side half of this problem and is a prerequisite here — see "Why this is cheap after 04" below.

The problem

database_options currently splits plot configuration between a per-datasource section (icca.grouped_fields, icca.loop, icca.spectrogram, icca.psd) and a global section (global.grouped_fields, global.loop). That split is not a concept anyone authors against — it is inconsistent across the three surfaces that touch it:

  • The XLSX never authors scope, it derives it. database_options_xlsx.py:373 places a group in global iff its signals span more than one datasource, otherwise in the datasource's own section. The spreadsheet author writes a group name in a groups column and never sees the distinction.
  • The two formats disagree on what is even expressible. The loops sheet (database_options_xlsx.py:398-410) has a datasource column and always writes a local loop — a global loop is unreachable from XLSX and can only be hand-written in JSON.
  • Two of the four sections have no global form at all. spectrogram and psd exist only per-datasource.

So "local vs global" is not one concept applied consistently. It is three different situations sharing a name.

Proposed change

Draw the seam where it actually falls — between config that describes how to read a source and config that describes what to plot:

  • Ingestion — field_display, signals, time_shift, … stay in the per-datasource section.
  • Presentation — grouped_fields, loop, spectrogram, psd all move to global, with every signal reference qualified (datasource::raw_name).

This retires the local/global distinction from the config surface entirely: there is one place to configure a plot, and one spelling for a signal reference.

Why this is cheap and safe after ticket 04

Ticket 04 makes assemble_plot_groups flatten every local section into qualified global references as its first step, so all assembly logic downstream sees only global, qualified refs. Once that lands, moving the config surface is purely a parser-and-docs change with no risk to assembly logic — the semantics are already unified and under test. Doing both at once would combine an XL config migration with a structural refactor in one change, where a failure in either is hard to attribute.

Known costs / open questions

  1. other's mid-load writeback. datasource/sources/other/find_load_format.py:322-326 injects grouped_fields and the derived sections into the section dict it was handed. Under a global-only presentation surface, other would have to mutate a dict it does not own — a wider mutation surface that cuts against ADR-0009's per-file scoping. Needs a design answer before implementing.
  2. Config locality regresses. A user configuring icca today finds everything about it in one section. This change splits ingestion from presentation, so they look in two places. Arguably the right trade (the two concerns genuinely differ), but it is a real loss.
  3. Authoring verbosity. A 10-signal group repeats icca:: ten times where the local form named the datasource once. Worth considering whether a per-group datasource: default key earns its keep.
  4. Backward compatibility. Every existing hand-written database_options.json needs migration, or the loader needs to keep accepting (and flattening) the old shape. Decide which — and if migration, whether a one-shot converter ships with it.

Blast radius

  • database_options_xlsx.py — output shape changes; the derived-scope block (:367-393) is deleted outright.
  • database_options_parser.py — normalization and validation follow the new shape.
  • datasource/sources/other/find_load_format.py — writeback retarget (see cost 1).
  • example/demo_database/database_options.{xlsx,json} — both regenerate; tests/unit/test_example_assets.py enforces parity.
  • tests/unit/test_database_options_xlsx.py — ~950 lines of wire-format assertions with zero cst. references; largely rewrites.
  • docs/user_guide/tutorial.md — the config reference section.

Overlaps deferred architecture ticket 08 (give database_options a parsed type); worth sequencing the two together if 08 is ever revived.

Acceptance criteria

  • grouped_fields, loop, spectrogram, psd are configured in exactly one place, with qualified references.
  • The XLSX scope-derivation rule is gone, not merely bypassed.
  • A global loop is expressible from the spreadsheet.
  • other's dynamically-derived sections land in the new shape without widening what it mutates (cost 1 resolved).
  • The old shape is either migrated by a shipped converter or still accepted, per the decision on cost 4.
  • example/demo_database/database_options.{xlsx,json} regenerated and at parity; tutorial updated.

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions