Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions kb/algo/reroot.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# Rerooting Algorithms

Generic, scoring-pluggable root search shared across commands. The search algorithm is decoupled from any particular objective through the `RootStats` trait, so the same machinery serves clock-based rerooting (variance-weighted regression, dated tips) and date-free rerooting (divergence variance).
Generic, scoring-pluggable root search currently used for date-free divergence rooting. The search algorithm is decoupled from a particular objective through the `RootStats` trait and is designed to accept clock sufficient statistics as well.

The clock command currently retains its own copy of the search in `clock/find_best_root/`; its migration contract is tracked in [kb/issues/N-reroot-clock-search-duplicates-generic-module.md](../issues/N-reroot-clock-search-duplicates-generic-module.md).
Clock rerooting still uses a parallel search in `clock/find_best_root/`; it does not yet share the generic engine. Its migration contract is tracked in [kb/issues/N-reroot-clock-search-duplicates-generic-module.md](../issues/N-reroot-clock-search-duplicates-generic-module.md).

Design rationale and the full derivation live in the proposals:

Expand Down
39 changes: 39 additions & 0 deletions kb/issues/H-app-napi-cancellation-is-process-global.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# N-API cancellation is process-global instead of task-local

Every N-API command task shares one static cancellation flag [`packages/app-napi/src/progress.rs#L8`](../../packages/app-napi/src/progress.rs#L8). Each progress sink reads that flag, and every macro-generated task resets it before starting [`packages/app-napi/src/progress.rs#L63`](../../packages/app-napi/src/progress.rs#L63) [`packages/app-napi/src/commands.rs#L64`](../../packages/app-napi/src/commands.rs#L64).

## Data flow

The exported `cancel()` operation writes one `CANCELLED: AtomicBool`. All six command task families construct `NapiProgressSink` values that read the same bit through `is_cancelled()`. Starting any macro-generated task calls `reset_cancel()` against that shared bit. `AncestralTaskNoop` follows a separate compute path and does not perform the same reset.

The atomic protects concurrent access to one value; it does not associate the value with a task identity or prevent one task from changing another task's control state.

## Failure mode

- Cancelling one task cancels every concurrent task.
- Starting a new task clears a cancellation request made for an older task.
- The no-op task path does not follow the same reset sequence.

## Required contract

Cancellation ownership must identify one job. Construct a task-local cancellation token, retain access to that token in the corresponding exported task handle or job registry, and inject it into that task's progress sink. Task startup must never mutate cancellation state owned by another task.

The transport contract must define:

- how jobs are identified;
- whether cancellation is idempotent;
- the terminal result reported for a cancelled job;
- what happens when cancellation arrives before start or after completion.

## Validation

- Run two concurrent tasks, cancel one, and prove the other continues.
- Cancel an older task while a new task starts and prove the request is not cleared.
- Exercise every generated command family and the no-op task through the same lifecycle contract.
- Verify success, failure, panic, and cancellation each produce exactly one terminal result.

## Related issues

- [H-app-transport-contracts-diverge-across-clients.md](H-app-transport-contracts-diverge-across-clients.md)
- [H-core-multi-client-architecture-library-purity.md](H-core-multi-client-architecture-library-purity.md)
- [kb/tickets/app-unify-web-desktop-and-typescript-transport-contracts.md](../tickets/app-unify-web-desktop-and-typescript-transport-contracts.md)
32 changes: 18 additions & 14 deletions kb/issues/H-app-transport-contracts-diverge-across-clients.md
Original file line number Diff line number Diff line change
@@ -1,28 +1,32 @@
# Application transport contracts diverge across clients

Web, desktop, Rust, OpenAPI, and generated TypeScript do not share one request/result contract.
Web, desktop, Rust, OpenAPI, and generated TypeScript do not share one request, event, and result contract.

## Evidence

- `struct ServerOptimizeArgs` contains reroot fields that are absent from the OpenAPI `OptimizeArgs` schema [packages/app-server/src/args.rs#L397-L456](../../packages/app-server/src/args.rs#L397-L456) [packages/app-contracts/openapi.yaml#L380-L409](../../packages/app-contracts/openapi.yaml#L380-L409).
- `fn command_sse()` emits command failures as `result` events [packages/app-server/src/sse.rs#L96-L120](../../packages/app-server/src/sse.rs#L96-L120), while `function createWebBridge()` resolves every result event [packages/app-web/src/bridge-web.ts#L40-L57](../../packages/app-web/src/bridge-web.ts#L40-L57).
- Desktop forwards JSON strings directly to command-specific N-API deserialization [packages/app-desktop/src/preload.ts#L29-L35](../../packages/app-desktop/src/preload.ts#L29-L35) [packages/app-napi/src/commands.rs#L76-L85](../../packages/app-napi/src/commands.rs#L76-L85).
- `struct ServerOptimizeArgs` contains reroot fields that are absent from the OpenAPI `OptimizeArgs` schema [`packages/app-server/src/args.rs#L397`](../../packages/app-server/src/args.rs#L397) [`packages/app-contracts/openapi.yaml#L380`](../../packages/app-contracts/openapi.yaml#L380).
- `fn command_sse()` emits command failures as `result` events, while `function createWebBridge()` resolves every result event as the promised result type [`packages/app-server/src/sse.rs#L93`](../../packages/app-server/src/sse.rs#L93) [`packages/app-web/src/bridge-web.ts#L40`](../../packages/app-web/src/bridge-web.ts#L40).
- Desktop forwards JSON strings directly to command-specific N-API deserialization [`packages/app-desktop/src/preload.ts#L29`](../../packages/app-desktop/src/preload.ts#L29) [`packages/app-napi/src/commands.rs#L76`](../../packages/app-napi/src/commands.rs#L76).
- Six server DTOs manually reconstruct nested command argument structs, duplicating fields and defaults from core and OpenAPI [`packages/app-server/src/args.rs#L21`](../../packages/app-server/src/args.rs#L21).
- Schema generation failure is reduced to a Cargo warning, allowing a successful build to retain stale contracts [`packages/app-cli/build.rs#L11`](../../packages/app-cli/build.rs#L11).
- Command defaults and option sets are repeated in core arguments, server arguments, and UI metadata, including timetree `max_iter` [`packages/treetime/src/commands/timetree/args.rs#L111`](../../packages/treetime/src/commands/timetree/args.rs#L111) [`packages/app-server/src/args.rs#L203`](../../packages/app-server/src/args.rs#L203) [`packages/app-ui/src/components/ParamForm.tsx#L15`](../../packages/app-ui/src/components/ParamForm.tsx#L15).

## Failures

- Optimize reroot fields exist in `ServerOptimizeArgs` but not in OpenAPI or generated `OptimizeArgs`.
- Web SSE sends command failures as `result` events; the TypeScript bridge resolves them as successful typed values.
- Desktop serializes flat transport DTOs directly into nested core command structs. `#[serde(default)]` can hide unknown/dropped fields.
- Optimize reroot fields exist in the server DTO but not in OpenAPI or generated TypeScript.
- Web SSE sends command failures through the success event name.
- Core serde defaults can accept absent fields while transport owners silently drift.
- A schema-generation error does not fail the build that consumes generated contracts.

## Potential solutions
## Required contract

- O1. One transport DTO/schema and one explicit conversion shared by web and desktop.
- O2. Separate client DTOs with independent converters and cross-client contract tests. This permits intentional differences but multiplies schema ownership.

## Recommendation

Define one canonical transport DTO and discriminated result envelope. Generate TypeScript from the same schema, deserialize the same DTO for web and desktop, and share one explicit conversion into core command arguments. Core structs are not wire formats.
One canonical transport model must own command names, request DTOs, result DTOs, progress/log events, cancellation, and a discriminated success/error/cancelled terminal envelope. HTTP/SSE and Electron/N-API remain transport adapters. Conversion from transport requests to application requests occurs once and rejects unknown or unrepresented fields.

## Related issues

- [H-core-multi-client-architecture-library-purity.md](H-core-multi-client-architecture-library-purity.md)
- [H-app-napi-cancellation-is-process-global.md](H-app-napi-cancellation-is-process-global.md)

## Related tickets

- [kb/tickets/app-unify-web-desktop-and-typescript-transport-contracts.md](../tickets/app-unify-web-desktop-and-typescript-transport-contracts.md)
75 changes: 19 additions & 56 deletions kb/issues/H-core-command-module-shared-ops-entanglement.md
Original file line number Diff line number Diff line change
@@ -1,65 +1,28 @@
# Command modules contain shared operations that belong in domain layers
# Command orchestration mixes application policy, domain workflow, and I/O

Command modules (`ancestral/`, `clock/`, `optimize/`, `commands/timetree/`, `prune/`) accumulated domain logic that multiple commands and core layers depend on. This creates a reverse dependency: core modules (`representation/`, `gtr/`, `cli/`, `test_utils`) import from `commands/`, violating the expected layering where commands compose domain logic, not define it.
The command boundary is not a thin adapter. Command runners combine argument translation, input loading, pipeline invocation, output policy, serialization, and filesystem writes, while the timetree pipeline concentrates most of the scientific workflow in one function.

This initial sweep is not exhaustive. More investigation and more reorganization will be needed as the codebase evolves.
## Evidence

## Cross-command production imports
- `fn run_ancestral_reconstruction()` reads FASTA, maps CLI arguments, runs inference, builds output projections, and writes several formats [`packages/treetime/src/commands/ancestral/run.rs#L31`](../../packages/treetime/src/commands/ancestral/run.rs#L31).
- The same application-level shape appears in clock, mugration, optimize, prune, and timetree runners under [`packages/treetime/src/commands`](../../packages/treetime/src/commands).
- `fn timetree::pipeline::run()` spans the complete scientific sequence from date loading and clock estimation through coalescent initialization, refinement, rerooting, confidence intervals, and result assembly [`packages/treetime/src/timetree/pipeline.rs#L102`](../../packages/treetime/src/timetree/pipeline.rs#L102).
- `fn run_refinement_iteration()` combines relaxed-clock application, topology resolution, partition reconciliation, ancestral-state comparison, and clock re-estimation [`packages/treetime/src/timetree/refinement.rs#L27`](../../packages/treetime/src/timetree/refinement.rs#L27).

Five cross-command dependency edges exist in production code (excluding tests):
Domain modules do not import `commands/`, but the remaining application orchestration has no explicit owner and the timetree workflow does not expose its scientifically meaningful state transitions.

- ~~`clock` -> `ancestral`: `MethodAncestral`~~ **Resolved**: extracted to `commands/shared/args`
- ~~`clock` -> `timetree`: `BranchLengthMode`, `RerootMethod`~~ **Resolved**: extracted to domain and shared command modules
- ~~`timetree` -> `ancestral` (6 sites)~~ **Resolved**: ancestral algorithms extracted to top-level `src/ancestral/`; `get_common_length` moved to `src/seq/alignment`
- ~~`timetree` -> `clock` (20 sites)~~ **Resolved**: clock model domain extracted to top-level `src/clock/`
- ~~`timetree` -> `optimize` (8 sites)~~ **Resolved**: optimization machinery extracted to top-level `src/optimize/`; iteration helpers to `packages/treetime/src/optimize/iteration.rs`
- ~~`optimize` -> `ancestral` (2 sites)~~ **Resolved**: ancestral algorithms extracted to top-level `src/ancestral/`; `get_common_length` moved to `src/seq/alignment`
- ~~`optimize` -> `prune` (1 site): `merge_shared_mutation_branches()`~~ **Resolved**: extracted to `optimize/topology/merge_shared_mutations`
- `homoplasy` -> `ancestral`: `TreetimeAncestralArgs` - remaining cross-command arg dependency (minor: CLI struct embedding)
## Open design question

## Reverse dependencies (core importing from commands)
The application operation boundary must serve CLI, HTTP, N-API, desktop, and future Python clients. Moving all orchestration into the CLI crate would leave the other clients without an owner. The unresolved choice is whether a shared application crate owns validated operations and in-memory results, or each adapter invokes domain pipelines directly through transport-neutral request types.

- ~~`partition/fitch_config.rs`, `partition/likelihood.rs` -> `get_common_length()` from `ancestral/fitch`~~ **Resolved**: moved to `seq/alignment`
- ~~`PartitionOptimizeOps` from `optimize/partition_ops`~~ **Resolved**: moved to `partition/traits.rs`
- ~~`PartitionRerootOps`, `PartitionTimetreeOps`, `PartitionTimetreeAll` from `commands/timetree/partition_ops`~~ **Resolved**: moved to `partition/traits.rs`
- ~~`RerootChanges` from `clock/reroot`~~ **Resolved**: extracted to `optimize/topology/reroot`
- ~~`ClockSet`, `ClockEdge`, `ClockNode`, `DateConstraintNode`, `TimetreeEdge`, `TimetreeNode` from `clock/` and `commands/timetree/`~~ **Resolved**: `ClockSet` moved to `payload/clock_set.rs`; traits moved to `payload/traits.rs`
- ~~`OptimizationContribution` from `optimize/optimize_unified`~~ **Resolved**: `OptimizationContribution` type + constructors moved to `partition/optimization_contribution.rs`; evaluation methods in `partition/optimization_contribution.rs` via split impl block
- ~~`ClockModel`, `ClockRegressionResult` from `clock/`~~ **Resolved**: clock model domain extracted to top-level `src/clock/`
- ~~`compress_sequences()`, `get_common_length()`, `initialize_marginal()`, `update_marginal()` from `ancestral/`~~ **Resolved**: ancestral algorithms extracted to top-level `src/ancestral/`; `get_common_length` moved to `src/seq/alignment`
No ticket should move `commands/` until this boundary is decided. Scientific stage ordering and fallback behavior must remain unchanged unless separately approved.

## Resolved categories
## Related issues

### ~~Ancestral reconstruction~~ **Resolved**

Extracted to top-level `src/ancestral/` module: `fitch.rs`, `fitch_indel.rs`, `marginal.rs`. `get_common_length` moved to `packages/treetime/src/seq/alignment.rs`. `ancestral/` retains only `args.rs` and `run.rs`.

### ~~Clock model domain~~ **Resolved**

Extracted to top-level `src/clock/` module: `clock_model.rs`, `clock_regression.rs`, `clock_filter.rs`, `clock_output.rs`, `clock_graph.rs`, `date_constraints.rs`, `reroot.rs`, `rtt.rs`, `assign_dates.rs`, `find_best_root/`. `clock/` retains only `args.rs` and `run.rs`.

### ~~Branch length optimization~~ **Resolved**

Extracted to top-level `src/optimize/` module: `dispatch.rs`, `optimize_indel.rs`, `method_brent.rs`, `method_newton.rs`, `optimize_eval.rs`, `optimize_dense_eval.rs`, `optimize_sparse_eval.rs`. Coefficient extraction types (`optimize_dense.rs`, `optimize_sparse.rs`, `optimization_contribution.rs`) moved to `partition/`. Iteration helpers (`save_branch_lengths`, `apply_damping`, topology cleanup) to `optimize/iteration.rs`. Domain arg types (`BranchOptMethod`, `InitialGuessMode`) to `optimize/args.rs`. `optimize/` retains only `args.rs` and `run.rs`.

### ~~Partition and payload traits~~ **Resolved**

Traits and types moved to `partition/` and `payload/`: partition traits (`PartitionOptimizeOps`, `PartitionRerootOps`, `PartitionTimetreeOps`, `PartitionTimetreeAll`) to `partition/traits.rs`; payload traits (`ClockNode`, `ClockEdge`, `DateConstraintNode`, `TimetreeNode`, `TimetreeEdge`) to `payload/traits.rs`; `ClockSet` data type to `payload/clock_set.rs`.

### ~~Topology operations~~ **Resolved**

`merge_shared_mutation_branches()` extracted to `optimize/topology/merge_shared_mutations`.

### ~~CLI args shared across commands~~ **Resolved**

`BranchLengthMode`, `RerootMethod`, `RerootSpec`, and `MethodAncestral` extracted to domain and shared command modules. `BranchOptMethod` and `InitialGuessMode` extracted to `optimize/args.rs`.

## Remaining items

- `homoplasy` -> `ancestral`: `TreetimeAncestralArgs` CLI struct embedding (minor)
- Output helpers: `write_graph()` duplicated across `ancestral`, `optimize`, `prune` (tracked separately, not yet filed)
- All commands delegate to `<module>/pipeline.rs` for pure computation. `commands/` (args + I/O handlers) remains in the library; planned move to consumer crates (`app-api`, `app-cli`) is not yet done

## Related tickets

- [kb/tickets/architecture-migrate-command-tests-with-dissolution.md](../tickets/architecture-migrate-command-tests-with-dissolution.md)
- [H-core-multi-client-architecture-library-purity.md](H-core-multi-client-architecture-library-purity.md)
- [H-app-transport-contracts-diverge-across-clients.md](H-app-transport-contracts-diverge-across-clients.md)
- [M-core-partition-init-orchestration-duplication.md](M-core-partition-init-orchestration-duplication.md)
- [M-command-output-ownership-is-scattered.md](M-command-output-ownership-is-scattered.md)
- [M-mugration-analysis-interface-exposes-policy-wiring.md](M-mugration-analysis-interface-exposes-policy-wiring.md)
- [M-timetree-refinement-iteration-mixes-state-transitions.md](M-timetree-refinement-iteration-mixes-state-transitions.md)
- [M-output-module-mixes-topology-ordering.md](M-output-module-mixes-topology-ordering.md)
Loading
Loading