refactor(delphi): UMAP/EVōC pipeline — library-first stages, in-process orchestration - #2627
Draft
jucor wants to merge 3 commits into
Draft
refactor(delphi): UMAP/EVōC pipeline — library-first stages, in-process orchestration#2627jucor wants to merge 3 commits into
jucor wants to merge 3 commits into
Conversation
Move the numbered stage scripts' logic into an importable stages/ package so the orchestrator can call them in-process, while keeping each stage runnable standalone from the CLI: 501_calculate_comment_extremity.py -> stages/comment_extremity.py 502_calculate_priorities.py -> stages/comment_priorities.py 700_datamapplot_for_layer.py -> stages/datamapplot_layer.py Each numbered script becomes a 3-6 line CLI shim (from stages.<name> import main; sys.exit(main())). Every stage main() now accepts an optional argv so it can be invoked in-process without mutating global sys.argv. run_pipeline.main gains the same argv parameter; reset_conversation gains a cli(argv) wrapper while keeping main(zid, rid) intact for existing callers/tests. Content-preserving: no algorithm or output changes. test_501 updated to import the moved module (patch targets follow the code). Full suite: 570 passed, 17 skipped, 47 xfailed (unchanged from baseline). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the Delphi UMAP/EVōC orchestration so delphi/run_delphi.py executes pipeline stages via importable, in-process entry points (with a subprocess escape hatch), while keeping the numbered stage scripts as thin CLI shims for backwards-compatible standalone execution.
Changes:
- Added a stage runner (
stages/runner.py) that normalizes in-process execution outcomes into subprocess-style exit codes and supportsDELPHI_STAGE_ISOLATION=subprocess. - Extracted stage logic into importable modules under
umap_narrative/stages/and converted501/502/700scripts into delegation shims. - Updated the orchestrator (
run_delphi.py) and key stages (run_pipeline.py,reset_conversation.py) to support argv-based entry points for in-process calls, plus added tests for dispatch and import hygiene.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| delphi/umap_narrative/stages/runner.py | Introduces in-process/subprocess stage execution abstraction and stage registry. |
| delphi/umap_narrative/stages/datamapplot_layer.py | New importable DataMapPlot stage module with main(argv) entry point. |
| delphi/umap_narrative/stages/comment_priorities.py | New importable priorities stage module with main(argv) entry point. |
| delphi/umap_narrative/stages/comment_extremity.py | New importable extremity stage module with main(argv) entry point. |
| delphi/umap_narrative/stages/init.py | Documents the new importable-stage layout and shim strategy. |
| delphi/umap_narrative/run_pipeline.py | Makes main() accept argv for in-process orchestration. |
| delphi/umap_narrative/reset_conversation.py | Adds cli(argv) wrapper so orchestration can call it without touching sys.argv. |
| delphi/umap_narrative/700_datamapplot_for_layer.py | Replaced full script with a thin shim delegating to stages.datamapplot_layer. |
| delphi/umap_narrative/502_calculate_priorities.py | Replaced full script with a thin shim delegating to stages.comment_priorities. |
| delphi/umap_narrative/501_calculate_comment_extremity.py | Replaced full script with a thin shim delegating to stages.comment_extremity. |
| delphi/tests/test_stage_runner.py | Adds test coverage for runner dispatch behavior, shim wiring, and import hygiene. |
| delphi/tests/test_501_calculate_comment_extremity.py | Updates test imports/patch targets to the new stage module location. |
| delphi/run_delphi.py | Switches orchestration from subprocess spawning to run_stage(...) calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+40
to
+55
| # Prepare arguments for the boto3 resource. | ||
| boto3_kwargs = { | ||
| 'region_name': os.environ.get('AWS_REGION', 'us-east-1'), | ||
| 'aws_access_key_id': os.environ.get('AWS_ACCESS_KEY_ID', 'dummy'), | ||
| 'aws_secret_access_key': os.environ.get('AWS_SECRET_ACCESS_KEY', 'dummy') | ||
| } | ||
|
|
||
| # Only add the endpoint_url if it's actually provided. | ||
| # If it's None (like in a production environment), boto3 will correctly | ||
| # use its default AWS endpoint resolution. | ||
| if endpoint_url: | ||
| boto3_kwargs['endpoint_url'] = endpoint_url | ||
|
|
||
| # Initialize DynamoDB connection using the prepared arguments | ||
| self.dynamodb = boto3.resource('dynamodb', **boto3_kwargs) | ||
|
|
Introduce stages/runner.py: a small Stage value type + run_stage(stage, argv, isolation) dispatcher and a STAGES registry. In-process is the default; DELPHI_STAGE_ISOLATION=subprocess restores per-stage subprocess execution as an escape hatch. run_stage converts a stage's outcome (return value, sys.exit, argparse error, or crash) into a subprocess-style exit code, so the orchestrator keeps its existing per-stage abort/warn control flow. The math pipeline (delphi/polismath/, owned by another workstream) is registered subprocess-only. Adds tests/test_stage_runner.py (25 tests): env-flag resolution, in-process vs subprocess dispatch, exit-code conversion, subprocess-only pinning, argparse isolation via sys.argv poisoning, no-network-at-import for each stage module, and shim->module delegation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tage Replace each subprocess.run(stage) in the orchestrator with run_stage(STAGES[...], argv). Stages now run in-process by default (heavy imports -- sentence-transformers, torch, umap, evoc, datamapplot -- paid once instead of once per subprocess), while DELPHI_STAGE_ISOLATION=subprocess restores the old behaviour. Content-preserving: identical stage order (reset -> math -> umap-pipeline -> extremity -> priorities -> datamapplot-per-layer), identical per-stage argv, identical error semantics (reset/math abort on nonzero; umap/extremity/priorities/ datamapplot warn-and-continue), and the DynamoDB layer-discovery block, env-var contracts (OLLAMA_MODEL/OLLAMA_HOST/MAX_VOTES/BATCH_SIZE/DELPHI_APP_PATH/PYTHONPATH/ DYNAMODB_ENDPOINT) and colored logging are all unchanged. Heavy stage deps are imported lazily, so --help and orchestrator import stay cheap. Full suite: 595 passed (570 baseline + 25 stage-runner), 17 skipped, 47 xfailed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jucor
force-pushed
the
jc/umap-inprocess-stages
branch
from
July 21, 2026 19:50
80fe512 to
49b8114
Compare
Delphi Coverage Report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactors the UMAP/EVōC pipeline orchestration from shell-subprocess calls to library-first in-process calls, per Julien's request: keep every stage independently runnable as a script (modularity for experiments), but have
run_delphi.pyimport and call stage entry points instead of spawning interpreters.What changed (3 commits)
umap_narrative/stages/gains importable modules (comment_extremity,comment_priorities,datamapplot_layer); the numbered scripts (501_…,502_…,700_…) become 3–6-line shims with byte-identical CLIs (verified--helpfor all).stages/runner.py: onerun_stage(stage, argv)function dispatching in-process (default) or subprocess (DELPHI_STAGE_ISOLATION=subprocessescape hatch, per stage). Stage outcome (return /sys.exit/ argparse error / crash) is normalized to a subprocess-style exit code so the orchestrator's error handling is unchanged. 25 new tests (tests/test_stage_runner.py): clean imports without network/heavy side effects, shim→module mapping, dispatch, argparse isolation.run_delphi.pycallsrun_stagewith byte-identical argv (before/after table in the commit body). Stage order, abort-vs-warn semantics, env contracts, and DynamoDB checkpointing are all preserved.Notes
polismath/run_math_pipeline.py(the math stage) stays subprocess-pinned — owned by the parity workstream; the runner treatstarget=Noneas subprocess-only.run_delphipasses--verboseto two stages that don't define it; argparse exit 2 flows through the same warn/skip path as before.🤖 Generated with Claude Code