Dataface Tasks

Scan chart code for hardcoded defaults config wrappers and unnecessary local aliases

IDDFT_CORE-SCAN_CHART_CODE_FOR_HARDCODED_DEFAULTS_CONFIG_WRAPPERS_AND_UNNECESSARY_LOCAL_ALIASES
Statuscompleted
Priorityp1
Milestonem1-ft-analytics-analyst-pilot
Ownersr-engineer-architect
Completed bydave
Completed2026-03-27

Problem

Audit chart compile/render code for hardcoded in-code values, get_some_value style config wrappers, and unnecessary local aliases like some_value = theme.some_value where direct chain access should be used, then clean them up to match the compiled-contract and anti-slop architecture.

Context

  • Background architecture:
  • ai_notes/refactors/DATAFACE_SPEC_PROFILE_AND_VEGA_LITE_STRATEGY.md
  • ai_notes/research/DATAFACE_AS_A_DASHBOARD_SPEC_FROM_VEGA_LITE_OUTWARD.md
  • ai_notes/refactors/CHART_DEFAULTS_AND_PRECEDENCE_AUDIT.md
  • The agreed direction is:
  • fully defined Pydantic language layer
  • YAML defaults/policy
  • canonical compiled contract
  • dedicated Dataface profile-mapping layer
  • thin Vega-Lite emitter
  • Even after the architectural seams are fixed, there is likely cleanup debt across chart compile/render code:
  • hardcoded fallback values that should come from compiled defaults or config
  • get_some_value() style wrappers that only proxy config/theme/structure access
  • unnecessary local aliases such as some_value = theme.some_value when direct chained access would be clearer
  • defensive/cowardly access patterns that should disappear once compiled contracts are trusted
  • This task is intentionally broader than existing config-consolidation work because it targets anti-slop cleanup patterns, not just one config file boundary.
  • Relevant likely scan areas:
  • dataface/core/render/chart/*
  • dataface/core/compile/*
  • helpers around theme/structure/chart defaults and presentation
  • Constraint:
  • do not delete aliases/wrappers that carry real logic or boundary value
  • do not guess defaults; if a value belongs in YAML/defaults, move it there or point at the compiled contract explicitly

Possible Solutions

  1. Fix obvious offenders opportunistically while doing other tasks. Cheap short term, but it leaves the codebase uneven and misses systematic patterns.
  2. Recommended: run a dedicated anti-slop/defaults audit after the mapping seam exists, then clean the codebase in one focused pass. This gives a single review surface for hardcoded values, wrappers, aliases, and direct-access cleanup.
  3. Add lint rules only. Useful later, but lint alone will not decide whether a wrapper/helper should exist at all.
  4. Ban all local aliases universally. Too blunt; some aliases improve readability when they collapse repeated complex expressions or carry real semantic naming.

Plan

Selected: Option 2 — do a dedicated anti-slop/defaults audit after the canonical mapping layer is in place, then clean chart compile/render code in one pass.

Files to modify: 1. chart compile/render modules with hardcoded fallback/default usage 2. helper modules with trivial getter/wrapper functions 3. tests that currently rely on implicit code-side fallbacks 4. docs/comments where config/compiled-contract access rules need restating

Implementation steps: 1. Inventory hardcoded fallback/default values still present in chart compile/render code. 2. Inventory trivial wrappers and helper functions that only proxy one config/theme path. 3. Inventory unnecessary local aliases that should be replaced by direct chained access. 4. For each finding, classify it as: - move default to YAML/config - read directly from compiled contract - delete wrapper and inline access - keep because it has real logic/boundary value 5. Add regression tests where removing a code-side fallback changes effective behavior. 6. Update comments/docs so future contributors know the cleanup rule set.

Implementation Progress

Audit findings (scan across dataface/core/render/chart/)

Duplicate utility functions (FIXED) - _to_plain_dict() duplicated in presentation.py and standard_renderer.py - _deep_merge() / _deep_merge_dicts() duplicated in presentation.py, standard_renderer.py, and as a local closure inside apply_presentation_defaults - Consolidated to single definitions in presentation.py, imported where needed

Hardcoded subtitle constants (FIXED) - KPI_SUBTITLE_FONT_DELTA, KPI_MIN_SUBTITLE_FONT_SIZE, KPI_SUBTITLE_GAP, KPI_SUBTITLE_CHAR_WIDTH_RATIO, KPI_VALUE_VERTICAL_RATIO in kpi.py - TABLE_SUBTITLE_FONT_DELTA, TABLE_MIN_SUBTITLE_FONT_SIZE, TABLE_SUBTITLE_BASELINE_OFFSET in table.py - SPARK_BAR_SUBTITLE_FONT_DELTA, SPARK_BAR_MIN_SUBTITLE_FONT_SIZE in spark_bar.py - All moved to default_config.yml under respective chart_types.* sections

Hardcoded table values duplicating existing config (FIXED) - absolute_min_col_width = 40.0table_config.absolute_min_col_width - char_w = 7.5table_config.content_char_width - + 16 (cell padding) → table_config.cell_padding_x * 2 - int(cw / 8) (header char width) → int(cw / table_config.header_char_width) - cell_x + 8 (cell padding) → cell_x + table_config.cell_padding_x - (header_height / 2) + 4+ table_config.text_baseline_offset - int(cw / 7) (data row char width) → int(cw / table_config.content_char_width) - width or 800width or table_config.width

Reviewed and kept (not slop) - Pipeline predicates (_needs_type_enrichment etc.) — readable named predicates, not wrappers - first_field() in spec_builders.py — used in 4+ places, provides semantic naming - _resolve_header_wrap() in table.py — real resolution logic with config fallback - _parse_column_width() in table.py — real parsing logic (% strings, int/float) - Local aliases in render_table_svg (e.g. row_height = table_config.row_height) — extracted to pass as args to helper functions, not pure aliases

Not addressed (geo/arc/decisions thresholds) - Geo projection centers/scales in geo.py — legitimate domain constants for built-in geo sources - Arc chart size - 100 / 100 minimums — layout arithmetic, not presentation defaults - Format selection thresholds in decisions.py (1_000_000, 10_000, 0.8) — semantic heuristic constants, not config defaults - Standard renderer padding (12, 5) — specific to horizontal bar label gutter math

QA Exploration

  • [x] QA exploration completed (or N/A for non-UI tasks)
  • N/A — no UI changes; all modifications are internal refactors with identical output

Review Feedback

  • [MEDIUM] _deep_merge docstring claimed no mutation but used shallow copy → updated docstring to accurately describe shallow-copy merge semantics
  • [MEDIUM] _deep_merge and _to_plain_dict used underscore prefix but were cross-module imports → renamed to to_plain_dict and overlay_merge
  • [HIGH] int(cw / 7) (content truncation) was conflated with content_char_width (7.5, column-width estimation) — split into separate content_truncation_char_width: 7.0 config key
  • [MEDIUM] deep_merge name implied mutation-safe deep copy — renamed to overlay_merge to reflect shallow-copy semantics
  • All fixes applied in follow-up commits

  • [x] Review cleared