Repository navigation
Proposal: staged behavior-preserving refactor of tool responsibility placement #1408
Description
Hi, I wanted to discuss a broader refactoring direction before continuing beyond the small pVACbind-local refactor PR #1407 I have already submitted against staging.
That PR is intentionally narrow. It reorganizes pvactools/tools/pvacbind/run.py by extracting pVACbind-local run-planning and handoff logic into a local run_planning.py. The goal is behavior preservation, not behavior change.
While working on that slice, I noticed a broader responsibility-placement pattern that may be worth improving gradually across a few tools. Several command runner files appear to combine multiple kinds of knowledge in one procedural coordinator, including:
CLI/application coordination;
tool-local run planning;
class-I/class-II execution decisions;
output layout decisions;
pipeline argument and handoff construction;
combined/report handoff construction;
post-processing setup;
repeated or implicit shared mechanics across tools.
I do not want to turn this into a rewrite or a large single PR. My preferred approach would be staged, reviewable, multiple behavior-preserving commits or PRs that move responsibilities to clearer local owners while preserving existing behavior and public contracts.
The goal would be to make future maintenance and extension safer by making responsibility placement more explicit: command run.py files would stay focused on orchestration and side effects, while tool-local planning and handoff decisions would live in named local modules/classes/functions where that improves readability and testability.
The constraints I would follow are:
keep each slice small and reviewable;
preserve behavior unless a behavior change is explicitly discussed;
avoid changing parser contracts, output schemas, prediction semantics, filtering/post-processing behavior, fixtures, or docs unless maintainers agree;
add or preserve characterization tests around each refactor slice;
avoid active bugfix/release areas;
rebase/adapt if relevant bugfixes merge first;
prefer tool-local responsibility placement first, unless maintainers want an early shared-abstraction discussion.
PR #1407 follows that model:
run.py remains the side-effect coordinator;
pVACbind-local planning and handoff logic is extracted locally;
prediction algorithm classification, allele classification, pipeline mechanics, PostProcessor semantics, report parsing, and output schemas are intended to remain unchanged;
preservation tests cover effective pipeline kwargs, run selection, output layout, combined-report handoff, and relevant guardrails.
Before going wider, I wanted to ask:
Are you open to staged, behavior-preserving responsibility-placement refactors across a few tools?
Should future slices stay tool-local first, or would you prefer an early discussion about shared abstractions across tools?
Are there specific tools, files, release plans, or open PRs I should avoid?
Given the branch policy, should this work continue from staging, while avoiding hotfix/bugfix areas unless you direct otherwise?
Would you prefer that I pause after PR #1407 until you decide whether this direction is useful?
I understand bugfixes and release work should take priority. If a bugfix touches the same area, I would treat that bugfix as the source of truth, rebase after it merges, and adapt the refactor to preserve the fixed behavior rather than asking bugfix authors to adapt to the refactor.
If this direction is not useful for the project right now, I can keep the work limited to PR #1407 or pause before touching other tools/shared mechanics.
Thank you.
Activity
susannasiebert commentedon Jun 10, 2026
@martin1cifuentes thank you for your interest in pVACtools and thank you for submitting a PR. Generally, we are very interested in code improvements like the on your are proposing. However, we are preparing for a major refactor of basically all of the pipelines so for the time being I think it might be better to hold of until this refactor is completed which might take a couple of months.
PRs related to the refactor are being merged into the 8.0.0 branch in order to keep the staging branch clear for any 7.x related feature additions. So I think your PRs would be best based off of the 8.0.0 branch.
martin1cifuentes commentedon Jun 10, 2026
Thank you for the context @susannasiebert , that makes sense. Im happy to hold off on this PR and rebase it against 8.0.0 instead.
I would also be interested in helping with the broader pipeline refactor if that would be useful, since its the kind of work which I am specializing on. While working on the pVACbind slice, I mapped parts of the code by responsability/knowledge ownership, this means, which parts of the system own run validity, accepted work handoff, prediction evidence, report meaning, output projection, etc.
My rough read was that some of the current complexity comes from temporal decomposition, for example, in run.py the code follows the order of execution while accumulating knowledge which belongs to more specific areas, which causes knowledge leakage by temporal decomposition, which is very common and natural but risky.
A very rough target shape I had in mind was something like:
pVAC tools
|
|----CLI / public command layer
| |-- pVAC-specific run commands + helpers
|
|----Use case service layer
| |---run pVACseq / pVACbind / pVACfuse / pVACsplice / pVACvector
| |__ launch/review with pVACview
| |__ compare reports with pVACcompare
| (Coordinate high-level intent, ask the right domain bodies for decisions/evidence, should | not own all child-domain truth)
|
|--- Domain bodies (Product knowledge)
| |---run validity / restart currentness, sequence and input evidence, prediction evidence, etc.
| |---report meaning, filtering /aggregation, vector construction, etc.
|
|---Projection / compatibility / infrastructure
|---output projection: TSV / FASTA / JSON / stdout / UI
|---compatibility: old, split, intermediate, or legacy output shapes
|---infrastructure: filesystem, subprocesses, APIs, downloads, persistence
So the goal would be to make each recurring piece of product knowledge have one clear owner, then the tools that need that knowledge can ask that owner instead of re encoding the same decision locally. The reason I think this kind of decomposition can help is that it addresses several common sources of long term complexity: low level mechanics accumulating in files that should express higher level intent, temporal decomposition where execution order objects also become knowledge owners, duplicated decisions across tools, and knowledge leaking into projections, compatibility outputs, fixtures, or infrastructure code. The goal would be to make the execution flow still easy to follow, but have the actual product knowledge owned by the body that understands it.
I also have a systematic process I use for approaching these refactors, if useful, id be happy to have a discussion about that and how it could fit with the refactor work already happening.
That said, I would be happy to go in whichever direction is most useful for you. If there is already an area of 8.0.0 where this kind of analysis or contribution would help, I would be glad to go there.
susannasiebert commentedon Jul 23, 2026
I have (mostly) completed the refactor I talked about. I think most of the issues that you are suggesting are addressed by that. Please have a look at the 8.0.0 branch and let me know if there are any additional suggestions you have to improve things.