Scan chart code for hardcoded defaults config wrappers and unnecessary local aliases
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.mdai_notes/research/DATAFACE_AS_A_DASHBOARD_SPEC_FROM_VEGA_LITE_OUTWARD.mdai_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_valuewhen 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
- Fix obvious offenders opportunistically while doing other tasks. Cheap short term, but it leaves the codebase uneven and misses systematic patterns.
- 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.
- Add lint rules only. Useful later, but lint alone will not decide whether a wrapper/helper should exist at all.
- 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.0 → table_config.absolute_min_col_width
- char_w = 7.5 → table_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 800 → width 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_mergedocstring claimed no mutation but used shallow copy → updated docstring to accurately describe shallow-copy merge semantics - [MEDIUM]
_deep_mergeand_to_plain_dictused underscore prefix but were cross-module imports → renamed toto_plain_dictandoverlay_merge - [HIGH]
int(cw / 7)(content truncation) was conflated withcontent_char_width(7.5, column-width estimation) — split into separatecontent_truncation_char_width: 7.0config key - [MEDIUM]
deep_mergename implied mutation-safe deep copy — renamed tooverlay_mergeto reflect shallow-copy semantics -
All fixes applied in follow-up commits
-
[x] Review cleared