perf(mobile-apps): speed up and harden Dataverse schema creation - #313
perf(mobile-apps): speed up and harden Dataverse schema creation#313Shubham Agarwal (kanu-shubham) wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request hardens and speeds up the mobile-apps Dataverse schema workflow by enforcing a fail-closed reconciliation phase before any metadata writes, and by ensuring new-table creation uses the fastest supported single-POST path with a validator guard.
Changes:
- Updated
/add-dataverseskill guidance to require full target reconciliation (with a global “no writes if any block” barrier) and to enforce two-pass ordering (ordinary columns first, relationships second). - Added
validate-table-create-payload.js(+ tests) to validate that new-table create payloads contain exactly the planned inline non-lookup columns before POSTing. - Updated the
data-model-architectagent instructions to classifyblockon unknown/unavailable target metadata and to forbid automatic replacement/guessing.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| plugins/mobile-apps/skills/add-dataverse/SKILL.md | Documents stricter reconciliation, a hard global write barrier, inline-create enforcement, and two-pass schema writes. |
| plugins/mobile-apps/scripts/validate-table-create-payload.js | New CLI validator to fail fast on incomplete/incorrect inline column sets for new table creates. |
| plugins/mobile-apps/scripts/tests/validate-table-create-payload.test.js | Unit tests for the new validator, including @file paths with spaces. |
| plugins/mobile-apps/agents/data-model-architect.md | Updates planning guidance to be target-grounded and fail closed with explicit Block outcomes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
plugins/mobile-apps/agents/data-model-architect.md:194
- The Step 6 introduction links to
${PLUGIN_ROOT}/skills/add-dataverse/references/data-architecture-reference.md, but that file does not exist in the mobile-apps plugin (onlyskills/add-dataverse/references/dataverse-reference.mdexists). This makes the tiering instructions point to a dead reference.
## Step 6 — Build Dependency Tiers
plugins/mobile-apps/agents/data-model-architect.md:33
- This bullet says the agent only classifies schema as
Reuse,Extend,Create, orBlock, but later in Step 5 you also introduceUnverifiedas a first-class decision. The hard-rules list should match the actual decision set to avoid conflicting instructions.
- **No automatic replacement.** This agent classifies schema as `Reuse`, `Extend`, `Create`, or `Block`. Replacing an existing table/column requires a separately approved migration with dependency analysis and data movement; it is outside this workflow.
plugins/mobile-apps/skills/add-dataverse/SKILL.md:368
- The self-check says to count
Attributes[]and compare it to the plan's total column count. If the plan lists lookups/relationships (or deferred calculated/rollup columns) alongside ordinary columns, this comparison will incorrectly fail even when the create payload is correct. The check should explicitly compare only against the planned ordinary (non-lookup, non-calculated/rollup, non-primary-id) columns plus the single primary-name attribute.
1. **Every planned ordinary column is present** — String, Memo, Integer, BigInt, Decimal, Money, DateTime, Boolean, Choice, MultiSelect Choice, Image, and File. Count `Attributes[]` and compare with the plan's column count for this table.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
plugins/mobile-apps/agents/data-model-architect.md:155
- The definitions of
BlockvsUnverifiedare internally inconsistent:Blockcurrently includes "target metadata is unavailable", butUnverifiedalso includes "a non-200/404 response". This ambiguity makes it unclear whether transient metadata query failures should halt planning (Block) or produce a draft plan (Unverified) as described elsewhere in this doc.
- **Reuse** — existing table fits as-is (no schema changes needed)
- **Extend** — existing table is the right concept, all same-name columns are compatible, missing columns are custom additions, and both `IsCustomizable.Value` and `CanCreateAttributes.Value` permit extension
- **Create** — the planned item is explicitly a new custom table, the exact logical name returns 404, the publisher prefix is verified, and all required-existing dependencies are present
- **Block** — target metadata is unavailable; a standard/managed/required-existing dependency is missing; the table cannot accept attributes; or any same-name table/column is incompatible
- **Unverified** — discovery could not run for this entity (Step 1/2 failure or a non-200/404 response). Record the intended decision plus the reason; `/add-dataverse` re-checks it before any write
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:64
- The fake Azure CLI shim is written only as an
azfile. On Windows,execFileSync('az', ...)will typically resolve viaPATHEXTtoaz.cmd/az.exe, so this test will not reliably intercept calls (and may fail or invoke the real Azure CLI if installed). Add anaz.cmdwrapper in the temp dir that forwards to the fake script so the tests run cross-platform.
function makeFakeAz(t) {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fake-az-'));
t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
const azPath = path.join(dir, 'az');
fs.writeFileSync(azPath, FAKE_AZ, { mode: 0o755 });
return { dir, logPath: path.join(dir, 'az.log') };
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:64
- The fake
azexecutable is written as a Unix shebang script namedaz. On Windows,execFileSync('az', ...)(used by getAuthToken) typically won’t execute that file, so these tests will fail even though the production code is fine. Add anaz.cmdshim in the temp dir that runs the script via the current Node binary so command resolution works cross-platform.
function makeFakeAz(t) {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fake-az-'));
t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
const azPath = path.join(dir, 'az');
fs.writeFileSync(azPath, FAKE_AZ, { mode: 0o755 });
return { dir, logPath: path.join(dir, 'az.log') };
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:64
- The auth-token-resolution tests create a fake
azbinary as a no-extension executable (<tmp>/az). On Windows,execFileSync('az', ...)resolves commands viaPATHEXT(e.g.az.cmd/az.exe), so this fake binary won’t be discovered and the test may accidentally invoke the real Azure CLI (making the test non-deterministic / requiring auth). Add a Windows-compatible wrapper (e.g.az.cmd) in the temp dir that forwards to the Node script so the tests are cross-platform and hermetic.
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fake-az-'));
t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
const azPath = path.join(dir, 'az');
fs.writeFileSync(azPath, FAKE_AZ, { mode: 0o755 });
return { dir, logPath: path.join(dir, 'az.log') };
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js:64
- The fake
azshim is written only as anazfile. On Windows,execFileSync('az', ...)resolves viaPATHEXT(e.g.,az.cmd/az.exe) and will not execute a bareazscript, so these tests will fail on win32. Write anaz.cmdwrapper (and a.jspayload) whenprocess.platform === 'win32'so PATH interception works cross-platform.
function makeFakeAz(t) {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fake-az-'));
t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
const azPath = path.join(dir, 'az');
fs.writeFileSync(azPath, FAKE_AZ, { mode: 0o755 });
return { dir, logPath: path.join(dir, 'az.log') };
}
| - **Reuse-first.** Always query existing tables and prefer reuse > extension > new. Don't propose a `cr123_customer` table if a standard `contact` table fits. | ||
| - **Reuse-first and target-grounded.** Query the exact target metadata for every proposed table, including standard tables, and prefer reuse > extension > new. Don't propose a `cr123_customer` table if a verified target `contact` table fits. | ||
| - **Never invent existing schema.** Never propose recreating or imitating a missing standard, managed, or solution-owned table/column. If discovery cannot run, you may still draft a plan from requirements, but mark it `Discovery skipped` so the user and `/add-dataverse` treat every decision as unverified — `/add-dataverse` re-reconciles against live metadata and blocks mutations there. | ||
| - **No automatic replacement.** This agent classifies schema as `Reuse`, `Extend`, `Create`, or `Block`. Replacing an existing table/column requires a separately approved migration with dependency analysis and data movement; it is outside this workflow. |
There was a problem hiding this comment.
Is block state necesary, what happens if user hits this block scenario?
|
|
||
| **If present:** read the `## Data Model` section. Extract: | ||
| - The reuse / extend / create table | ||
| - The target reconciliation table (`reuse` / `extend` / `create` / `block` decisions and evidence) |
There was a problem hiding this comment.
block state can be removed?
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
plugins/mobile-apps/skills/add-dataverse/SKILL.md:215
- The reconciliation matrix currently treats a failed metadata read as
create/"Proceed". If discovery cannot confirm the target state, creating is not a safe default; it should be deferred/unverified so the workflow doesn't mutate schema based on unknown state.
| Batched query failed (non-2xx) after retry and per-table split | `create` | unknown | Proceed; Step 5a's POST-time collision rescue resolves it by extend or rename. |
plugins/mobile-apps/skills/add-dataverse/SKILL.md:48
- Step 2 only calls out
reuse/extend/create/adapt/deferdecisions, but the data-model-architect instructions in this PR introduceblock/unverifiedoutcomes. If the plan contains those,/add-dataverseneeds to carry them forward explicitly (and treat them as non-mutating) to avoid silently proceeding with an unresolved state.
**If present:** read the `## Data Model` section. Extract:
- The target reconciliation table (`reuse` / `extend` / `create` / `adapt` / `defer` decisions and evidence)
- The Mermaid ER diagram (informational)
- The "Creation Order" tier list
Carry forward any `adapt` (auto-renamed) and `defer` (out-of-scope this run) decisions with their recorded reasons, and apply the alias map to every name you use. A data-modelling conflict never halts this skill — it resolves to `adapt` or `defer` and is reported in Step 9.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
plugins/mobile-apps/skills/add-dataverse/SKILL.md:215
- Step 4 says any name still unreadable after retry+split is classified
defer(and the PR description emphasizes fail-closed reconciliation), but the matrix row for a batched-query failure currently sets the table decision tocreateand proceeds. That contradicts the preceding guidance and can lead to unverified metadata writes when discovery is unavailable.
| Batched query failed (non-2xx) after retry and per-table split | `create` | unknown | Proceed; Step 5a's POST-time collision rescue resolves it by extend or rename. |
plugins/mobile-apps/agents/data-model-architect.md:150
- This decision list omits
Defer, but the surrounding text and examples useDeferfor missing standard/managed/required-existing dependencies. As written, readers have to infer whetherDeferis a valid state or whetherBlock/Unverifiedshould be used instead, which can cause inconsistent plans.
- **Reuse** — existing table fits as-is (no schema changes needed)
- **Extend** — existing table is the right concept, all same-name columns are compatible, missing columns are custom additions, and both `IsCustomizable.Value` and `CanCreateAttributes.Value` permit extension
- **Create** — the planned item is explicitly a new custom table, the exact logical name returns 404, the publisher prefix is verified, and all required-existing dependencies are present
- **Block** — target metadata is unavailable; a standard/managed/required-existing dependency is missing; the table cannot accept attributes; or any same-name table/column is incompatible
- **Unverified** — discovery could not run for this entity (Step 1/2 failure or a non-200/404 response). Record the intended decision plus the reason; `/add-dataverse` re-checks it before any write
| - **Reuse-first.** Always query existing tables and prefer reuse > extension > new. Don't propose a `cr123_customer` table if a standard `contact` table fits. | ||
| - **Reuse-first and target-grounded.** Query the exact target metadata for every proposed table, including standard tables, and prefer reuse > extension > new. Don't propose a `cr123_customer` table if a verified target `contact` table fits. | ||
| - **Never invent existing schema.** Never propose recreating or imitating a missing standard, managed, or solution-owned table/column. If discovery cannot run, you may still draft a plan from requirements, but mark it `Discovery skipped` so the user and `/add-dataverse` treat every decision as unverified — `/add-dataverse` re-reconciles against live metadata and adapts or defers anything that conflicts. | ||
| - **No automatic replacement.** This agent classifies schema as `Reuse`, `Extend`, `Create`, `Adapt` (create beside a conflicting object under a new name), `Defer` (leave out of this run), or `Unverified` (target metadata could not be read). Replacing an existing table/column requires a separately approved migration with dependency analysis and data movement; it is outside this workflow. A data-modelling conflict is never a blocker — it is an `Adapt` or a `Defer` with a recorded reason. |
There was a problem hiding this comment.
Do we need so many states, Reuse, Extend, Create sounds enough
Summary
This PR improves Dataverse schema-provisioning performance while preserving the same generated data model, relationships, security behavior, and offline capabilities.
The optimized workflow created 8 tables and 10 lookup relationships in 4 minutes 17 seconds, compared with 5 minutes 56 seconds for the baseline.
What changed
The Dataverse creation workflow was streamlined to reduce metadata-provisioning overhead:
The optimization affects provisioning time only. The resulting Dataverse schema remains functionally equivalent.
Benchmark scope
Both runs used the same:
Fresh logical names were used for the optimized run to avoid idempotent skips and deleted-table tombstones.
Results
All 8 optimized-run tables were queried after creation and confirmed to exist in Dataverse.
Throttling-adjusted comparison
The baseline encountered one 30-second Dataverse throttling delay. The optimized run did not show a comparable delay.
After excluding that delay:
This indicates that the result is not explained only by the throttling event, although Dataverse service variability can affect individual runs.
Leadership impact
The observed improvement reduces developer waiting time during:
Projected savings at the observed rate:
Safety and compatibility
The performance improvement does not trade away correctness:
Validation
Conclusion
The initial like-for-like benchmark shows that the optimized Dataverse provisioning workflow is approximately 28% faster, or approximately 21% faster after accounting for baseline throttling.
This is a meaningful improvement for developer productivity and automated provisioning while retaining equivalent schema output and Dataverse safety controls.
Because this comparison currently represents one run per variant, the result should be treated as an initial performance signal. Repeated runs should use the median and slowest execution times for a production performance commitment
Cut wall-clock time out of the mobile-app Dataverse workflow, and add a fail-closed reconciliation step before any metadata write.
Three independent speedups:
az account showon every Dataverse call.2Nper-table discovery requests with one filtered query.1. Auth Short-Circuit
getAuthToken()built its tenant candidates as an array literal:A JavaScript array literal evaluates every element eagerly, before
.filter()runs. So the challenge probe andaz account showexecuted on every single call, even when an env var already supplied the tenant and would have won the loop.Measured on a warm macOS box:
az account showaz account get-access-token(warm)Candidates are now produced lazily and the loop stops at the first token. Preference order, de-duplication, and the unqualified-token fallback are unchanged — only redundant work is skipped. For reference,
power-pagesissues a singleaz account get-access-tokenper call and pays ~0.46s of overhead./add-dataversenow also exportsPOWER_PLATFORM_TENANT_IDonce from the valueresolve-environment.jsalready returns, so later calls short-circuit at the first candidate and skip the challenge probe entirely.2. Batched Metadata Reads
Discovery previously issued one entity GET plus one attributes GET per planned table. Both collapse into a single documented query:
This is the documented way to query multiple table definitions at once. A name present in
value[]exists; a name absent does not — which is exactly the signal the reconciliation matrix needs. The expanded$selectstays on baseAttributeMetadataproperties because one query cannot cast to a derived column type.Two follow-on reductions fall out of that:
Tombstones and hidden collisions were never reliably visible to the pre-flight anyway — the POST-time collision rescue remains the real safety net, and is unchanged.
3. Create-Path Shape
New tables: one
EntityDefinitionsPOST carrying the primary name plus every planned ordinary column, guarded by a five-point self-check (completeness, no inline lookup/customer/owner metadata, no deferred or server-owned columns, exactly one primary-name attribute, no duplicates). Lookups and relationships are a deliberate second pass.Existing tables: compare against the cached snapshot, then POST only missing columns sequentially — replacing one preflight GET per planned column.
Metadata writes stay strictly sequential (Dataverse serializes them behind an exclusive lock) and no metadata
$batchis introduced.Reconciliation Before Writes
Every planned table and column is classified against live target metadata as
reuse,extend,create, orblock, usingIsManaged,IsCustomizable, andCanCreateAttributes. A global barrier requires zero blocked items before Step 5 begins.Missing standard or managed dependencies are
blockwith remediation to install/import the owning solution — never recreated as a custom imitation.replaceis deliberately not an automatic state, since Dataverse cannot change a column type in place.Planning stays non-blocking: if discovery cannot run, the architect still drafts a plan, marks it
Discovery skipped, flags entitiesUnverified, and returnsDONE_WITH_CONCERNS./add-dataversere-queries live metadata and refuses to write what it cannot confirm.No New Validator Script
An earlier revision added a
validate-table-create-payload.jsvalidator. It was removed: the expected-column list came from the same agent that produced the payload, so it could not independently prove plan alignment, and it compared only names — not types, formats, or lengths. The rule is now a self-check in the skill.dataverse-request.jsremains the only script in that path.Estimated Impact
For a model with ~7 planned tables (5 new, 2 extended) and 6 lookups, Dataverse calls drop from roughly 45 to 21. Combined with removing
az account showfrom every call, per-call overhead falls from ~5.6s to ~0.5s.Derived from the measured per-operation costs above rather than an end-to-end benchmark, that is on the order of 3–4 minutes of overhead removed per data-model run. The column-inlining change additionally removes up to
C-1writes per new table when the agent would otherwise have created a table shell.Files
plugins/mobile-apps/scripts/lib/validation-helpers.js— lazy tenant resolution.plugins/mobile-apps/scripts/tests/auth-token-resolution.test.js— new regression tests.plugins/mobile-apps/skills/add-dataverse/SKILL.md— batched discovery, snapshot-based pre-flight, batched verify, reconciliation matrix, write barrier, create self-check, two-pass ordering.plugins/mobile-apps/agents/data-model-architect.md— batched target query, target-grounded classification, non-blocking discovery failures.Validation
node --test plugins/mobile-apps/scripts/tests/— 19 passedgetAuthTokento the eager version makes the newno az account showtest fail; the fix makes it pass.getAuthTokensignature unchanged.validate-skill-descriptions/ensure-skill-version-check/validate-keyword-case/validate-plugin-names/validate-legacy-compatibility/validate-telemetry-ikeys— all pass.git diff --check— clean.No live Dataverse environment was mutated during validation.
Compatibility and Risk
getAuthTokenremains re-callable for 401 refresh (no memoization).$filterURL would grow impractically long, the skill splits it into a few queries — still far fewer than one per table.Rollback
Revert the commits on this branch. No persisted runtime format changes and no repository migration.