Repository navigation
Traversal v1 (2/4): the labeled traversal walker - #709
adamant-pwn wants to merge 49 commits into
Conversation
Second piece of the labeled-traversal v1 stack: extend a seed under a permitted label set, rebuilt from Gunnar's walker on gr/labeled-traversal (d5d26ba) for the v1 feature set only: - the seed's labels named explicitly (labels not carrying every seed k-mer are dropped and reported) or derived as the labels on every seed k-mer, column or header, capped with a dropped count and digest; - left, right or both arms, level-synchronous; - per path the labels carrying every k-mer walked; a label ends with dead_end, label_lost, branch (over its branch allowance), edge_reuse, edge_reuse_rc or rejoined_seed; hairpins are skipped; - a tree of segments per arm (paths are kept apart where they meet); - the extension length, the time budget and fixed safety caps (steps, live paths, paths, output bases, splits per path), with complete_to_bp stating where every admissible walk is present. Not carried over: label switching and loss budgets, trace support and record coordinates, annotate mode, memory and work budgets, read pacing, the lookahead, the attempt protocol, reconvergence merging, beams, quorums, continuations, growth profiles and branch-event refusals. Tests: the walker suite trimmed to the v1 features, and a check of every walker claim against an independent string-level model of the records (test_trie_reference.hpp: label_trie, from Gunnar's trie reference). Source paths on d5d26ba: metagraph/src/graph/traversal/walker.{hpp,cpp} metagraph/tests/graph/traversal/test_walker.cpp metagraph/tests/graph/traversal/test_trie_reference.hpp metagraph/tests/graph/traversal/test_trie_cases.cpp (cases) Co-authored-by: Gunnar Rätsch <Gunnar.Ratsch@ratschlab.org> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
max_label_branches "unlimited" excluded every label on two or more successors, the opposite of what it means: an unlimited allowance never excludes a label. Found by the parity check against the prototype (ndm1_unlimited: 10 left paths instead of 29). Co-authored-by: Gunnar Rätsch <Gunnar.Ratsch@ratschlab.org> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GCC 12 (-Warray-bounds, -Werror) reports assigning a one-element initializer list to a std::vector<uint64_t> as reading past the list; assign(1, x) says the same without the temporary array. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
resolve_label now refuses an ambiguous name. Derivation checks the same two conditions directly, so an ambiguous derived header still ends the seed with ambiguous_header instead of failing the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A column without sequence records threw out_of_range when the walker derived header labels from a seed. Skip it, as discovery does. The derivation test's explicit request for a header of two records now expects the refusal of an ambiguous name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A path that reached max_splits_per_path ended its labels with BRANCH
("split_limit"), and the arm still reported COMPLETE with complete_to_bp
at the full extension, although the walks past that fork were never
explored. The split limit is a safety cap: end the cut labels and the path
with MAX_SPLITS ("max_splits_per_path"), mark the arm TRUNCATED and lower
complete_to_bp to the fork.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After the first k-mer, derivation copied every column (or every record of a live column) of each row before intersecting. A row with many columns, or a repetitive k-mer with many coordinates, then grew the working set far past the live labels. Keep only live labels while reading a row; the intersection is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The deadline was read only per head, after the fetch of the whole level's rows, so a slow fetch could overrun the budget by a full level. Read it before the fetch as well, ending the heads as the per-head check does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The allowance-0 reference check passed even if the walker ignored the allowance. Also require that a label whose reference walks part ends with BRANCH, and that every reference walk is covered by a claim on its prefix. Add a deterministic inverted-repeat case for EDGE_REUSE_RC. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
output_bp counted the same bases as steps, and max_steps (1,000,000) is below max_output_bp (2,000,000), so MAX_OUTPUT could never stop a walk. Remove the cap, its end reason and the counter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Length, characters and named labels can be checked before any walk. traverse_seed runs the same function first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Review completed against the latest diff
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Architecture diagram
sequenceDiagram
participant API as Traversal Service API
participant Walk as traverse_seed()
participant Oracle as LabelOracle
participant Graph as DeBruijnGraph
participant Anno as AnnotatedDBG
participant WalkRule as Walker Engine
participant Caps as Cap Manager
participant Result as SeedResult
Note over API,Result: Labeled Traversal Walker Flow
API->>Walk: Request traversal with Seed + Strategy
Walk->>Walk: validate_seed()
alt Seed not in graph
Walk-->>API: SeedNotInGraph error
end
Walk->>Oracle: Create LabelOracle(graph)
Oracle->>Graph: Get k-mer size and alphabet
Oracle-->>Walk: Oracle instance
alt Explicit labels provided
Walk->>Oracle: Check labels carry seed k-mers
else Derive labels from seed
Walk->>Oracle: Query annotations for seed k-mers
Oracle->>Anno: Fetch label annotations
Anno-->>Oracle: Label hits per k-mer
Oracle-->>Walk: Derived label set
alt No carrier label found
Walk-->>API: SeedDerivationError (NO_CARRIER)
end
end
Walk->>WalkRule: Initialize arms (LEFT/RIGHT per direction)
WalkRule->>Graph: Get seed boundary k-mers
Graph-->>WalkRule: Boundary nodes
loop Walk level by level (max_extension_bp)
WalkRule->>WalkRule: Generate successor candidates
WalkRule->>Graph: Query successors for current k-mer
Graph-->>WalkRule: Succ list with base chars
WalkRule->>WalkRule: Apply walk rule per candidate
Note over WalkRule: Check edge reuse, seed re-entry, hairpins
WalkRule->>Caps: Check resource limits
Caps-->>WalkRule: Status (OK or TRUNCATED)
alt Cap exceeded
WalkRule->>Result: Mark cap trigger + lower complete_to_bp
else All caps fine
WalkRule->>WalkRule: Split paths at label branches
WalkRule->>WalkRule: Track label ends with reasons
WalkRule->>Result: Extend segments and paths
end
alt Label criteria per branch
WalkRule->>WalkRule: Filter successors by label carry
WalkRule->>WalkRule: Apply branch allowance
WalkRule->>WalkRule: End labels with DEAD_END/LABEL_LOST/BRANCH
end
end
WalkRule->>Result: Build segment tree with splits
Result->>Result: Record path ends and label ends
Result-->>API: Complete SeedResult with arms
Shadow auto-approve: would not auto-approve because issues were found.
View guided diff | Turn on auto-fix | Re-trigger cubic
It was defined inside the file's anonymous namespace, which made the call ambiguous with the declaration in walker.hpp and broke the build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Refuse a non-finite time budget and zero step, path or extension caps. - Leave complete_to_bp at 0 for an arm that was not requested. - Check the resubmittability of derived header labels only for the labels kept under max_seed_labels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Adds the labeled traversal walker with reference tests; the decisive unresolved issue is cubic's dismissed P2 that non-finite time_budget_ms can disable the time-budget safety cap, with no fix visible in the diff.
View guided diff | Turn on auto-fix | Re-trigger cubic
# Conflicts: # metagraph/src/graph/traversal/traversal_types.cpp # metagraph/src/graph/traversal/traversal_types.hpp
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Adds the labeled traversal walker with an independent reference model and caps; the decisive unresolved issue is the dismissed P2 that non-finite time_budget_ms can disable the time-budget safety cap, with no fix visible in the diff.
View guided diff | Turn on auto-fix | Re-trigger cubic
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Adds the labeled traversal walker with an independent reference model and revised label discovery; the dismissed P2 that non-finite time_budget_ms can disable the time-budget safety cap is still not addressed in the visible diff.
View guided diff | Turn on auto-fix | Re-trigger cubic
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Adds the labeled traversal walker with an independent reference test model. The dismissed P2 that NaN/infinite time_budget_ms can disable the TIME_BUDGET safety cap is still unaddressed, so accepting that risk needs human sign-off.
View guided diff | Turn on auto-fix | Re-trigger cubic
- Explicit seed labels are checked with resolve_support instead of a copy of its run building; DroppedLabel::runs are KmerIntervals. - Successor keys come from AnnotationKeyMapper; seed keys from map_to_nodes. - Edge hashes use folly::hash::fnva64_buf; the seed's missing k-mers are reported with encode_presence_mask; seeds are case-mapped with in_graph_case. - The walker tests use the shared traversal test helpers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 6 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Adds the labeled traversal walker with bounded caps and an independent reference test. The dismissed P2 that NaN/infinite time_budget_ms can disable the TIME_BUDGET safety cap is still unaddressed, making acceptance of that safety-cap risk the decisive open question.
View guided diff | Turn on auto-fix | Re-trigger cubic
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would require human review. Adds the labeled traversal walker with bounded caps and an independent reference test. The decisive open issue is the safety cap: non-finite time_budget_ms can disable the TIME_BUDGET cap and no fix is visible, so accepting that risk needs human review.
View guided diff | Turn on auto-fix | Re-trigger cubic
Second of four stacked PRs for labeled traversal, on #708.
This PR adds
traverse_seed, which extends a seed through the graph and keeps, on each path, the labels that carry every k-mer walked. The labels are named, or taken from the seed: those on every seed k-mer. A seed that no label carries fails withSeedDerivationError, so a caller can report it for that seed alone. Each arm returns a tree of segments, one path per leaf, the reason each label left, andcomplete_to_bp: the length up to which every walk is present.A path never reuses an edge or steps back into the seed, so walks end on graphs with cycles. A label that continues on several successors spends one of its
max_label_branches(default 0). With none left, it ends there. Fixed caps bound the work: 1,000,000 steps and a time budget per seed, 1000 live paths and 10,000 paths per arm, and 64 splits per path. A cap marks the arm truncated.test_walker_referencechecks every label end against a string model of the test records.Co-authored-by: Gunnar Rätsch
🤖 Generated with Claude Code