[FIX] correct dry-run counts via planned-remap chaining by chandrasekharan-zipstack · Pull Request #20 · Zipstack/unstract-python-client · GitHub
Skip to content

[FIX] correct dry-run counts via planned-remap chaining - #20

Merged
chandrasekharan-zipstack merged 2 commits into
mainfrom
fix/clone-dry-run-remap-chaining
Jun 18, 2026
Merged

[FIX] correct dry-run counts via planned-remap chaining#20
chandrasekharan-zipstack merged 2 commits into
mainfrom
fix/clone-dry-run-remap-chaining

Conversation

@chandrasekharan-zipstack

@chandrasekharan-zipstack chandrasekharan-zipstack commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

What

  • Fix unstract clone --dry-run so phase counts faithfully predict a real run. Adds a planned-remap mechanism so dependent phases plan-count without writing.

Why

  • Dry-run under-counted every phase downstream of a fresh create: a phase that would create a parent recorded no target id, so each dependent phase saw an empty remap and silently no-op'd — tool_instance, workflow_endpoint, pipeline, api_deployment reported 0 against a fresh target even though a real run creates them. It only looked right when the target already had the parents (the adopt path records a remap).
  • Dry-run also bucketed would-creates as skipped, so totals never matched the real run a dry-run exists to predict.

How

  • "Reads run, writes stub" contract — dry-run executes every read + dependency check, stubs only the POST/PATCH.
  • RemapTable.record_planned() mints a deterministic (uuid5) synthetic target id so dependent phases resolve the FK and plan-count; is_planned() flags them; snapshot(hide_planned=) masks them in the report.
  • Every create-capable phase's dry-run branch now counts in the bucket a real run would (created/adopted) and records a planned remap (group, adapter, connector, tag, custom_tool, files, workflow, tool_instance, workflow_endpoint, pipeline, api_deployment).
  • custom_tool runs its source-side validations (frictionless-adapter check, source-registry lookup → planned prompt_studio_registry remap) in dry-run so the plan reflects real create-vs-skip.
  • Phases doing live target lookups (tool_instance, workflow_endpoint, files) guard on is_planned — a synthetic parent id has no row on target, so they short-circuit instead of querying it (otherwise workflow_endpoint would count every endpoint as failed).
  • Report shows a DRY RUN banner; synthetic ids never reach the wire.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • No. Real-run (non-dry-run) behavior is unchanged: every change is inside an if dry_run branch, except two safe reorders that leave the real-run sequence identical — workflow_endpoint resolves the connector before the dry-run gate (real run: resolve → patch, same as before), and custom_tool._create_fresh moves the dry-run gate after the source-side validations (real run: validate → import, same as before). New report/remap fields default off and only affect dry-run. Full suite (incl. all non-dry happy-path tests) is green.

Database Migrations

  • None.

Env Config

  • None.

Relevant Docs

  • None.

Related Issues or PRs

  • Foundation (§7) for the planned cloud-entity clone support design.

Dependencies Versions

  • None (uses stdlib uuid only).

Notes on Testing

  • uv run pytest192 passed.
  • Updated per-phase dry-run tests to the new contract (created/adopted + no writes + planned remap recorded).
  • New coverage: RemapTable planned mechanics (determinism, is_planned, hide_planned masking) and planned-parent guards for tool_instance / workflow_endpoint (the end-to-end chain that previously regressed to 0).

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

https://claude.ai/code/session_011ja9H1rnSXmPUgQtHm8TNS

Dry-run under-counted every phase downstream of a fresh create: a phase
that would create a parent recorded no target id, so dependent phases saw
an empty remap and no-op'd (tool_instance/endpoint/pipeline/api_deployment
showed 0 on a fresh target). Dry-run also counted would-creates as
`skipped`, so totals didn't match the real run it's meant to predict.

Fix centrally with a "reads run, writes stub" contract:
- RemapTable.record_planned() mints a deterministic synthetic target id so
  dependent phases resolve the FK and plan-count without writing;
  is_planned() flags them; snapshot(hide_planned=) masks them in the report.
- Every create-capable phase's dry-run branch now counts in the bucket a
  real run would (created/adopted) and records a planned remap.
- custom_tool runs its source-side validations (frictionless check, source
  registry lookup) in dry-run so the plan reflects real create-vs-skip.
- Phases doing live target lookups (tool_instance, workflow_endpoint, files)
  guard on is_planned to avoid querying a synthetic parent id.
- Report shows a DRY RUN banner; synthetic ids never reach the wire.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011ja9H1rnSXmPUgQtHm8TNS
@chandrasekharan-zipstack chandrasekharan-zipstack changed the title fix(clone): correct dry-run counts via planned-remap chaining [FIX] correct dry-run counts via planned-remap chaining Jun 18, 2026
@greptile-apps

greptile-apps Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Comment thread src/unstract/clone/phases/custom_tool.py
Address review: _create_fresh recorded the planned remap and _clone_one
re-recorded the same value, while the adopt path only recorded in
_clone_one. Drop the generic record in _clone_one; each sub-path
(adopt / fresh / fresh-dry-run) now records once, since only it knows
whether the target id is real or a planned synthetic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011ja9H1rnSXmPUgQtHm8TNS
@chandrasekharan-zipstack
chandrasekharan-zipstack merged commit cd9bcf5 into main Jun 18, 2026
3 checks passed
@chandrasekharan-zipstack
chandrasekharan-zipstack deleted the fix/clone-dry-run-remap-chaining branch June 18, 2026 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants