docs(prd): Demo Data Source & Source-of-Truth Hygiene
Open-source Excel-native EPM and consolidation for SAP & Dynamics
Brought to you by:
konsolid-at
Menu
▾
▴
#48 docs(prd): Demo Data Source & Source-of-Truth Hygiene
closed
nobody
None
2026-06-19
2026-06-15
Anonymous
No
Originally created by: pyy3
Summary
New design PRD (no code) for a seeded demo_data connector as the default ERP source — fixing the root issue we traced: a fresh install has 0 Connector docs, so erp_sources falls back to a hardcoded ["d365_fo"] that has no data, producing empty gold models. Two declared fixtures (Connector, Dimension Mapping) also ship without files.
What the PRD proposes
demo_dataas a first-class, seeded source (connector fixture +staging/demo_dataadapter) so a clean install is a working end-to-end demo with zero external config.erp_sourcesbecomes truly registry-derived — replace… or ["d365_fo"]with… or [].- Required empty-union guard in the 7 canonical staging models (deleting the last connector must not compile to invalid SQL).
- Dimension Mapping fixtures = crosswalk source of truth;
dimension_mappings.csvis a generated mirror; guard_regenerate_dimension_mappings_seed()to no-op on 0 docs. - Generated-artifact policy:
dbt_project.ymlvars + crosswalk are mirrors of the committed fixtures, CI-enforced (needs a new build-from-fixtures mode in the regenerator). - Deferred per-ERP dbt-package add-ons behind a locked, schema-tested canonical column contract.
Decisions
All six design open-questions are resolved (see Resolved Decisions in the doc): demo crosswalk (raw + mappings), generated-file tracking (tracked + CI sync), empty erp_sources (allowed + warn), template default ([]), add-on packaging (defer + contract), demo legal entities (fixed small set).
Scope of this PR
Docs only — docs/prd/PRD-DEMO-DATA-SOURCE.md (new) + docs/prd/README.md (index row under Phase 3). Implementation is a separate handoff. Acceptance Criteria (12) and an affected-components list are included for the implementer.
🤖 Generated with Claude Code
Originally posted by: pyy3
Review — accuracy + consistency pass
Reviewed against the actual code in
grynn-in/konsol(app) andgrynn-in/konsolidat(dbt). Docs-only PR, so this checks factual claims and internal consistency, not code correctness.✅ Every concrete claim verifies
dbt_config.py:243→erp_sources = _build_erp_sources_vars() or ["d365_fo"]connector.pyon_update/on_trashboth callregenerate_vars()(lines 32–41)erp_typeSelect has nodemo_datad365_fo,d365_bc,sap_s4,sap_ecc,sap_b1,erpnextconnector.jsonanddimension_mapping.jsonfixtures missing; others presentdimension.json/measure.jsonpresentafter_migratecalls_regenerate_dimension_mappings_seed()unconditionallyinstall.py:31){% for erp in erp_sources %}uniond365_foadapters / 7erpnextadaptersstg_gl_entries.sql:15→var('erp_sources', ['d365_fo']){% for erp in [] %}→with unioned as ( ))So nothing in the PRD is factually wrong, and the empty-union guard (§3) is correctly flagged as required, not optional.
🔶 Finding 1 (substantive) — Decision [#4]b (the seed guard) contradicts the existing design and Decision [#4]a
The PRD's Decision [#4]b / §4 / AC#6 says: guard
_regenerate_dimension_mappings_seed()to no-op when 0 published docs, "so a migrate can never empty a committed crosswalk."But the function's own docstring already documents the opposite as intentional (
dbt_config.py:45-51):It only skips when the seeds dir is absent; with 0 published docs it deliberately writes a header-only CSV.
Two problems:
Why the original problem actually disappears without the guard: the root cause was 0 docs because the fixture was missing. Decision [#4]a fixes that by shipping
dimension_mapping.json— and Frappe syncs fixtures beforeafter_migrateruns, so by the time the regenerator fires, the published docs exist and it writes the correct non-empty CSV. The fixture (#4a) solves it; the guard (#4b) is redundant at best, harmful at worst.Recommendation: drop Decision [#4]b / AC#6, OR re-scope it to the genuinely narrow case it might be defending — fixture-load failure silently emptying a previously-populated crosswalk (a different condition than "0 docs"). Either way, reconcile AC#6 with the existing, documented "empty-is-valid / pass-through" semantics so the PRD doesn't ask the implementer to reverse an intentional design.
🔹 Minor
regenerate_vars(notregenerate_dbt_vars) throughout. ✓frappe.get_all); flagged in the PRD, just confirming it's real work, not a rename.Verdict
Accurate and internally consistent except for the [#4]a/#4b crosswalk-guard conflict above — that one should be reconciled before implementation, since it currently asks the implementer to fight an existing intentional behaviour.
Disclosure: this is a self-review by the PRD's author (automated). It re-verifies the code claims and consistency objectively, but a second independent reviewer is still worthwhile for the design judgement (esp. the [#4] reconciliation).
Related
Tickets:
#2Tickets:
#4Originally posted by: pyy3
Independent review panel — 3 fresh-context reviewers
To offset author bias on my earlier self-review, I ran three independent reviewers (each given only the PRD + the repos, none of my prior reasoning): one for claim accuracy, one for design soundness, one devil's advocate. Consolidated, deduped, by severity. The panel found a likely-fatal issue the self-review missed.
🔴 CRITICAL — approach-level (devil's advocate)
C1 — A canonical-only
staging/demo_dataadapter cannot fill the gold models; the bronze layer is hardwired tod365_fo. Only GL + budget flow through the 7 canonical union models. ~11 bronze modelsref('stg_d365_fo__*')directly (fiscal calendars, financial dimensions/values, consolidation account groups, rate types, account categories, journal entries, budget register entries…).d365_fohas 16 staging files; the canonical contract covers only 7. A demo adapter that "mirrors the canonical column set" (Scope §1) produces none of the other 9 → those bronze models, and any gold depending on them, stay empty. This breaks AC#2 ("gold non-empty on fresh install") — the PRD's central promise. The proposed adapter is wired to the wrong seam.C2 — A simpler solution already exists on this branch, unreferenced by the PRD.
clickhouse/demo-data.sql+scripts/generate_demo_data.py(committed ondocs/prd-demo-data-source) populate the 14epm_rawAirbyte-shaped tables with a synthetic 3-entity group, and the table set exactly matchesmodels/staging/d365_fo/_d365_fo__sources.yml. Loading that lets the existing 16-filed365_foadapter consume it unchanged → the entire bronze→silver→gold pipeline lights up, anderp_sources=[d365_fo]becomes correct, not a defect.deploy.sh:265already runsdbt build. This dominates the PRD: no new adapter, no newerp_type, no crosswalk rows, no canonical-contract schema test, no CI drift machinery. Recommended alternative: load the existingdemo-data.sqlintoepm_raw+ ship ad365_foConnector fixture, instead of building a paralleldemo_dataadapter.🟠 HIGH
H1 — Decision #4 guard contradicts #4, the documented behavior, and introduces a bug. (Independently confirmed by 2 reviewers + my self-review.)
regenerate_dimension_mappings_seed()'s docstring (dbt_config.py:45-55) documents 0-docs→header-only-CSV as intentional ("empty file is valid — everything passes through"); harmonization is aLEFT JOIN ... on status='Published'(macros/dimension_helpers.sql), so empty = pass-through, not breakage. If fixtures are the source of truth (#4a), 0 docs → empty crosswalk is correct. The guard would break the legitimate "operator clears all mappings" flow (stale CSV retained). The actual Problem-#3 defect is fixed by #4a alone (ship the fixture). Drop or narrow #4(b)/AC#6.H2 — The fixture-load-vs-
after_migrateordering is load-bearing but only asserted, never verified. The "ship the fixture and the regenerator sees docs" argument depends on Frappe syncing fixtures beforeafter_migratefires. The PRD lists this as an affected component but never states the conclusion. Pin it down — it's the fact the whole [#4] story rests on.H3 — Lifecycle is
after_delete, NOTon_trash(factual error, cited as load-bearing). The PRD says regenerate is called "from bothon_updateandon_trash" and "Connector.on_trash→regenerate_vars()". The code (connector.py:34,41) useson_update+after_delete, with an explicit comment thaton_trashwould be a bug (runs before the row is deleted → connector still inerp_sources). The PRD cites the exact pattern the code was written to avoid, as proof "the plumbing already exists." Fix toafter_delete. (Line range32–41is correct.)🟡 MEDIUM
Connector/Dimension Mappingare declared as bare-string fixtures (nofilters) inhooks.py. On a customer site,export-fixtureswould pull their real connectors/mappings into the app — and AC#5's "round-trips without loss" is then unachievable. Specify filtered fixtures (e.g.erp_type=demo_data/ name prefix).d365_fo/erpnextrows must be migrated intodimension_mapping.json, else the drift check fails immediately.dbt_project.ymlmakes the dbt repo non-standalone — the property Decision [#2] exists to protect — and makes AC#9 non-testable. Decide whether the CI check is in-scope for this PRD.test_connector_registry.py:51(assertserp_typeis exactly the six) and:97(asserts the literalor ["d365_fo"]fallback). Both change; unmentioned.🟢 LOW
regenerate_dimension_mappings_seed()(dbt_config.py:45);_regenerate_dimension_mappings_seed()(install.py:48) is only the best-effort wrapper. Specify which gets the guard (wrapper = migrate path only; public fn = all callers).entities_loaded/Connector-Health coupling is asserted, not traced.Parts all reviewers agree are good (keep regardless of approach)
demo_datais adopted).d365_fo(Alt A/B), filtered, not a newdemo_datatype.Panel verdict
Not ready to implement as-is. Beyond the [#4] reconciliation and the
on_trash→after_deletefix, the panel's strongest result is C1+C2: because the bronze layer is hardwired tod365_fo, a canonical-onlydemo_dataadapter won't deliver a working demo — and an already-built, lower-maintenance alternative (loaddemo-data.sqlintoepm_raw+ ship ad365_foconnector fixture) exists on this branch. Recommend re-evaluating the core approach before implementation.Method: 3 independent fresh-context agents. C1/C2 in particular were missed by the author's self-review — which is exactly why the independent pass was run.
Related
Tickets:
#2Tickets:
#4Tickets:
#6Originally posted by: grynn-in
Second review pass (docs-only) · verdict: needs-changes
I read the existing review panel first and did not re-list its findings (H1 guard conflict, C1 bronze-hardwired-d365_fo, C2, H2 ordering, H3
on_trash→after_delete, M1–M4). Confirmed those are accurate. The current diff is unchanged, so the prior H1 finding is still unaddressed — Decision-guard/§4/AC#6 ("no-op when 0 published docs") still contradictsregenerate_dimension_mappings_seed()'s own docstring ("empty file is valid — everything passes through") and the Frappe-is-SoT decision.New issues this pass adds:
[MED · consistency] "Decision [#4]a/#4b" is unresolvable in the doc. There are two same-date decision sections with incompatible numbering: a "Decisions (locked …)" section with 2 unnumbered bullets, and a "Resolved Decisions" section numbered [#1]–#6 where #4 is the
var('erp_sources', [])template default — NOT the seed guard. The guard (called "#4b" in review) has no stable id. Merge into one numbered section so cross-refs resolve.[MED · house style] Missing the standard metadata header. Siblings open with bold
**Status:** / **Date:** / **Phase:** / **Repos:**; this uses a single italic*Status: …*line and omits Date/Phase/Repos. Add the 4-field header.[LOW] No
## Open Questionssection (house skeleton); README row uses an off-vocabulary status ("Design — ready" vs the usual "Not Started"/"✅ Implemented"); README "Last updated" still 2026-06-13 though this adds a row.[correction to existing review] C2 frames
clickhouse/demo-data.sql+scripts/generate_demo_data.pyas "on this branch" — verified they're pre-existing onorigin/main; this branch adds only the 2.mdfiles. The substance stands (14 demo tables match the 14 d365_fo sources) but it's not a contribution of this PR.All blockers are docs-only fixes in the two
.mdfiles (plus the separate konsolon_trash→after_deletecode fix the first pass flagged).Related
Tickets:
#1Tickets:
#4Originally posted by: pyy3
Decision recorded in fd68a8b:
demo_dataconnector chosen over the standalonedemo-data.sql(which leaves theerp_sourcesfallback unfixed). Combined with the earlier reconciliation commit (guard contradiction resolved, decision sections merged, house-style header + Open Questions, README aligned), the PRD is internally consistent and house-style compliant.LGTM — good to merge as a design doc. (Implementation is a separate PR per the Affected-components list.)
Ticket changed by: grynn-in