Repository navigation
fix(search,validate): exact-ID rank guarantee across modes + parent-hierarchy cycle detection (GH-281, GH-280) - #282
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JDymqzh8VsuJzFqtit5PD
… + detect parent-hierarchy cycles Two agent-trust correctness fixes for pm's core read/validate primitives, surfaced from dogfooding the large 2d-bots tracker (pm-rkie). GH-281 (pm-oqgf): exact-ID search was only guaranteed #1 in keyword mode. combineHybridHits() normalizes keyword scores to [0,1] and blends them under hybrid_semantic_weight, so an exact full/short-ID match's keyword boost (EXACT_ID_MATCH_SCORE=1000 / SHORT_ID_MATCH_SCORE=900) was capped at the keyword weight and could be out-ranked by a high-semantic body mention; in pure semantic mode an exact-ID item with no vector hit vanished entirely. Added an exact_id_match flag on SearchHit and forceExactIdHitsToTop(), applied at both the semantic and hybrid return points (after rerank), which reattaches exact-ID hits in a reserved band above every blended hit (full-ID above short-ID) and exempts them from the score threshold, so the exact target is always #1 and is never dropped — in every mode. GH-280 (pm-8vul): pm validate ran Tarjan SCC only over the dependency graph (blocked_by/definition_of_ready) and never traversed the parent composition graph, so a parent cycle (A<->B, or any-length ring) reported dependency_cycle_count:0 with no warning while silently breaking pm list --tree, orphan detection, and rollups. Added buildLifecycleParentGraph (child->parent over ALL items) + detectLifecycleParentCycles reusing the generic findLifecycleDependencyCycleComponents helper. Folds parent_cycle_* diagnostics into the lifecycle check, emits validate_hierarchy_parent_cycle[_error] warnings, adds --parent-cycle-severity off|warn|error (default warn, mirrors dependency-cycle policy) with full contract/help/completion wiring, and a remediation registry entry (pm update <id> --parent <other-id>). Coverage 100/100/100/100 (4536 tests); quality:static + contracts:check green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JDymqzh8VsuJzFqtit5PD
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
@coderabbitai full review |
|
/gemini review |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
✅ Action performedFull review finished. |
|
Warning Review limit reached
More reviews will be available in 25 minutes and 1 second. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughTwo correctness fixes are implemented: GH-281 adds an ChangesGH-281: Exact-ID Search Ranking Fix
GH-280: Parent Hierarchy Cycle Detection in pm validate
Agent PM Feature and History Metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces core read and validate integrity fixes. It guarantees that exact-ID search matches rank first across all search modes (hybrid, semantic, and keyword) by preserving an exact-ID match flag and forcing these hits to the top of the results. Additionally, it adds parent-hierarchy cycle detection to pm validate using Tarjan's SCC algorithm, along with a new --parent-cycle-severity flag and remediation registry entries. Feedback on these changes suggests resolving parent IDs case-insensitively to prevent missing cycle edges due to casing mismatches, and replacing the spread operator in Math.max with Array.prototype.reduce to avoid potential stack overflow errors on large search result sets.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function buildLifecycleParentGraph(items: ItemWithBody[]): Map<string, string[]> { | ||
| const knownItemIds = new Set(items.map((item) => item.id)); | ||
| const graph = new Map<string, string[]>(); | ||
| const sortedItems = [...items].sort((left, right) => left.id.localeCompare(right.id)); | ||
| for (const item of sortedItems) { | ||
| const edges: string[] = []; | ||
| const parentId = toMeaningfulString(item.parent); | ||
| if (parentId && knownItemIds.has(parentId)) { | ||
| edges.push(parentId); | ||
| } | ||
| graph.set(item.id, edges); | ||
| } | ||
| return graph; | ||
| } |
There was a problem hiding this comment.
Since parent matching was updated to be case-insensitive in PR #279, using a case-sensitive Set (knownItemIds) and knownItemIds.has(parentId) can fail to detect cycle edges if the parent reference casing does not exactly match the canonical item ID casing (e.g., parent: PM-FK49 vs id: pm-fk49). To ensure all cycle edges are correctly captured, we should resolve parent IDs case-insensitively to their canonical IDs.
function buildLifecycleParentGraph(items: ItemWithBody[]): Map<string, string[]> {
const canonicalIdMap = new Map<string, string>();
for (const item of items) {
canonicalIdMap.set(item.id.toLowerCase(), item.id);
}
const graph = new Map<string, string[]>();
const sortedItems = [...items].sort((left, right) => left.id.localeCompare(right.id));
for (const item of sortedItems) {
const edges: string[] = [];
const parentId = toMeaningfulString(item.parent);
if (parentId) {
const canonicalParentId = canonicalIdMap.get(parentId.toLowerCase());
if (canonicalParentId) {
edges.push(canonicalParentId);
}
}
graph.set(item.id, edges);
}
return graph;
}There was a problem hiding this comment.
| } | ||
| const exactIdIds = new Set(exactIdHits.map((hit) => hit.item.id)); | ||
| const remaining = rankedHits.filter((hit) => !exactIdIds.has(hit.item.id)); | ||
| const maxRemainingScore = Math.max(0, ...remaining.map((hit) => hit.score)); |
There was a problem hiding this comment.
Using the spread operator (...) with Math.max on an array of arbitrary size can cause a stack overflow (RangeError: Maximum call stack size exceeded) if the array is very large. Since search results can potentially be large before truncation, it is safer to use Array.prototype.reduce to find the maximum score.
| const maxRemainingScore = Math.max(0, ...remaining.map((hit) => hit.score)); | |
| const maxRemainingScore = remaining.reduce((max, hit) => Math.max(max, hit.score), 0); |
There was a problem hiding this comment.
Code Review
This pull request introduces two core read and validate integrity improvements: parent-hierarchy cycle detection in pm validate (GH-280) and an exact-ID search rank guarantee across hybrid and semantic modes (GH-281). The validation update adds a --parent-cycle-severity flag and leverages Tarjan's SCC algorithm to identify composition loops across all items, while the search update ensures exact full-ID and short-ID matches are prioritized at the top of results. The review feedback is highly constructive, identifying a potential case-sensitivity bug in the parent graph builder that could lead to missed cycles, and recommending robustness improvements in the search ranking helper to avoid call stack size errors on large datasets and preserve original search hit metadata.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function buildLifecycleParentGraph(items: ItemWithBody[]): Map<string, string[]> { | ||
| const knownItemIds = new Set(items.map((item) => item.id)); | ||
| const graph = new Map<string, string[]>(); | ||
| const sortedItems = [...items].sort((left, right) => left.id.localeCompare(right.id)); | ||
| for (const item of sortedItems) { | ||
| const edges: string[] = []; | ||
| const parentId = toMeaningfulString(item.parent); | ||
| if (parentId && knownItemIds.has(parentId)) { | ||
| edges.push(parentId); | ||
| } | ||
| graph.set(item.id, edges); | ||
| } | ||
| return graph; | ||
| } |
There was a problem hiding this comment.
Since case-insensitive parent matching was introduced and accepted (as noted in the PR description and commit c48f6782), building the parent graph using a case-sensitive Set lookup (knownItemIds.has(parentId)) can lead to false negatives. If an item's parent reference uses a different casing than the target item's ID, the edge will be silently ignored, and parent cycles involving that edge won't be detected.
Using a case-insensitive lookup map (e.g., mapping lowercase IDs to their actual IDs) correctly resolves parent edges regardless of casing.
function buildLifecycleParentGraph(items: ItemWithBody[]): Map<string, string[]> {
const idLowerToActual = new Map(items.map((item) => [item.id.toLowerCase(), item.id]));
const graph = new Map<string, string[]>();
const sortedItems = [...items].sort((left, right) => left.id.localeCompare(right.id));
for (const item of sortedItems) {
const edges: string[] = [];
const parentId = toMeaningfulString(item.parent);
if (parentId) {
const actualParentId = idLowerToActual.get(parentId.toLowerCase());
if (actualParentId) {
edges.push(actualParentId);
}
}
graph.set(item.id, edges);
}
return graph;
}There was a problem hiding this comment.
| const maxRemainingScore = Math.max(0, ...remaining.map((hit) => hit.score)); | ||
| // Full-ID hits (score 1000) must rank above short-ID hits (score 900): rank | ||
| // within the exact-ID band by the original keyword score (descending) so a | ||
| // full-ID match always precedes a short-ID match for the same query, then | ||
| // offset the whole band above every remaining hit. ids are unique, so two | ||
| // exact-ID hits can never tie on score — sorting on score alone is total. | ||
| const orderedExactHits = [...exactIdHits].sort((left, right) => right.score - left.score); | ||
| const bandBase = maxRemainingScore + orderedExactHits.length + 1; | ||
| const promoted = orderedExactHits.map((hit, index) => ({ | ||
| item: hit.item, | ||
| // Higher band slot for earlier (higher original keyword score) hits. | ||
| score: bandBase - index, | ||
| matched_fields: ["id"], | ||
| exact_id_match: true, | ||
| })); |
There was a problem hiding this comment.
There are two improvement opportunities in this function:
- Robustness & Memory Efficiency: Spreading a mapped array into
Math.max(i.e.,Math.max(0, ...remaining.map(...))) can trigger aRangeError: Maximum call stack size exceededif theremainingarray is very large, and it creates an unnecessary intermediate array. Usingreduceis safer and more memory-efficient. - Property Preservation: Re-creating the promoted hit objects manually drops other properties of
SearchHit(such asmatched_all_terms: truewhich was explicitly set inscoreDocument). Spreading the originalhit(i.e.,{ ...hit, score: ... }) ensures all metadata is preserved and makes the code more future-proof.
const maxRemainingScore = remaining.reduce((max, hit) => Math.max(max, hit.score), 0);
// Full-ID hits (score 1000) must rank above short-ID hits (score 900): rank
// within the exact-ID band by the original keyword score (descending) so a
// full-ID match always precedes a short-ID match for the same query, then
// offset the whole band above every remaining hit. ids are unique, so two
// exact-ID hits can never tie on score — sorting on score alone is total.
const orderedExactHits = [...exactIdHits].sort((left, right) => right.score - left.score);
const bandBase = maxRemainingScore + orderedExactHits.length + 1;
const promoted = orderedExactHits.map((hit, index) => ({
...hit,
// Higher band slot for earlier (higher original keyword score) hits.
score: bandBase - index,
matched_fields: ["id"],
}));![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/unit/commands/search-command.spec.ts (2)
2887-2911: 💤 Low valueSame variable shadowing pattern as the first test.
The
targetvariable inside the fetch mock shadows the outer test fixture. Same suggestion applies: rename tofetchTargetfor consistency and clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/commands/search-command.spec.ts` around lines 2887 - 2911, The `target` variable declared inside the fetch mock function shadows an outer test fixture variable with the same name, reducing code clarity. Rename the `target` variable to `fetchTarget` throughout the fetch mock function (where it is assigned from `resolveFetchTarget(url)` and used in the conditional checks with `endsWith()`). This change will maintain consistency with the naming convention used elsewhere and eliminate variable shadowing.
2729-2756: 💤 Low valueVariable
targetshadows the outer test fixture.The
const target = resolveFetchTarget(url)inside the fetch mock shadows the outertargetfixture variable defined at line 2685. While technically correct due to different scopes, it reduces readability. Consider renaming tofetchTargetorresolvedUrlfor clarity.♻️ Suggested fix
globalThis.fetch = (async (url: unknown) => { - const target = resolveFetchTarget(url); - if (target.endsWith("/v1/embeddings")) { + const fetchTarget = resolveFetchTarget(url); + if (fetchTarget.endsWith("/v1/embeddings")) { return { ok: true, // ... } as unknown as Response; } - if (target.endsWith("/collections/pm_items/points/search")) { + if (fetchTarget.endsWith("/collections/pm_items/points/search")) { return { // ... } as unknown as Response; } - throw new Error(`Unexpected fetch target: ${target}`); + throw new Error(`Unexpected fetch target: ${fetchTarget}`); }) as typeof globalThis.fetch;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/commands/search-command.spec.ts` around lines 2729 - 2756, The variable `target` inside the fetch mock function shadows the outer `target` fixture variable, reducing code clarity. Rename the local variable from `target` to `fetchTarget` or `resolvedUrl` throughout the fetch mock function, updating both the `const target = resolveFetchTarget(url)` assignment and all subsequent references to this variable in the endsWith() method calls that check for "/v1/embeddings" and "/collections/pm_items/points/search" endpoints.src/cli/commands/search.ts (1)
1980-1984: 💤 Low value
exact_id_matchis not stripped from output, contradicting the interface comment.The
SearchHitinterface comment (line 242) statesexact_id_matchis "Not projected into output rows," butprojectSearchHitsonly stripsmatched_all_terms. For consistency with the GH-181 pattern and the documented contract,exact_id_matchshould also be excluded from full-mode output.♻️ Suggested fix
function projectSearchHits(hits: SearchHit[], projection: SearchProjectionConfig): SearchResultItem[] { if (projection.mode === "full") { // matched_all_terms is an internal ranking signal (GH-181); strip it from - // full-mode output rows so the public hit shape stays { item, score, matched_fields }. - return hits.map(({ matched_all_terms: _matchedAllTerms, ...hit }) => hit); + // full-mode output rows so the public hit shape stays { item, score, matched_fields }. + // exact_id_match is likewise internal (GH-281). + return hits.map(({ matched_all_terms: _matchedAllTerms, exact_id_match: _exactIdMatch, ...hit }) => hit); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/commands/search.ts` around lines 1980 - 1984, The projectSearchHits function currently only strips matched_all_terms from the hit objects in full mode, but according to the SearchHit interface documentation, exact_id_match should also not be included in output rows. Update the destructuring pattern in the hits.map call within projectSearchHits to exclude both matched_all_terms and exact_id_match from the returned hit objects, similar to how matched_all_terms is already being stripped using the destructuring assignment pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/cli/commands/search.ts`:
- Around line 1980-1984: The projectSearchHits function currently only strips
matched_all_terms from the hit objects in full mode, but according to the
SearchHit interface documentation, exact_id_match should also not be included in
output rows. Update the destructuring pattern in the hits.map call within
projectSearchHits to exclude both matched_all_terms and exact_id_match from the
returned hit objects, similar to how matched_all_terms is already being stripped
using the destructuring assignment pattern.
In `@tests/unit/commands/search-command.spec.ts`:
- Around line 2887-2911: The `target` variable declared inside the fetch mock
function shadows an outer test fixture variable with the same name, reducing
code clarity. Rename the `target` variable to `fetchTarget` throughout the fetch
mock function (where it is assigned from `resolveFetchTarget(url)` and used in
the conditional checks with `endsWith()`). This change will maintain consistency
with the naming convention used elsewhere and eliminate variable shadowing.
- Around line 2729-2756: The variable `target` inside the fetch mock function
shadows the outer `target` fixture variable, reducing code clarity. Rename the
local variable from `target` to `fetchTarget` or `resolvedUrl` throughout the
fetch mock function, updating both the `const target = resolveFetchTarget(url)`
assignment and all subsequent references to this variable in the endsWith()
method calls that check for "/v1/embeddings" and
"/collections/pm_items/points/search" endpoints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9579f314-0de5-4751-9217-3ca87fd49cdf
📒 Files selected for processing (20)
.agents/pm/features/pm-6bz1.toon.agents/pm/features/pm-rkie.toon.agents/pm/history/pm-6bz1.jsonl.agents/pm/history/pm-8vul.jsonl.agents/pm/history/pm-oqgf.jsonl.agents/pm/history/pm-rkie.jsonl.agents/pm/issues/pm-8vul.toon.agents/pm/issues/pm-oqgf.toonsrc/cli/commands/completion.tssrc/cli/commands/search.tssrc/cli/commands/validate.tssrc/cli/help-content.tssrc/cli/register-operations.tssrc/core/diagnostics/remediation.tssrc/sdk/cli-contracts.tstests/fixtures/contracts/full.jsontests/unit/commands/completion-command.spec.tstests/unit/commands/register-commands.spec.tstests/unit/commands/search-command.spec.tstests/unit/commands/validate-command.spec.ts
… resolution + Math.max stack safety - validate.ts (HIGH): buildLifecycleParentGraph resolves parent refs via a lowercase->canonical id map so a casing mismatch (PR #279 made parent matching case-insensitive) can never silently drop a cycle edge and hide a parent cycle. Added a regression assertion (uppercase PM-PA resolves to pm-pa). - search.ts (MEDIUM): forceExactIdHitsToTop uses reduce instead of Math.max(0, ...spread) to avoid a RangeError on a very large pre-truncation candidate set, and promoted hits spread the original keyword hit so matched_all_terms / exact_id_match are preserved. Coverage 100/100/100/100 (4536 tests); static + contracts green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JDymqzh8VsuJzFqtit5PD
|
/gemini review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Code Review
This pull request introduces two core read and validate integrity improvements. First, it guarantees that exact-ID search matches always rank first in hybrid and semantic search modes by preserving an exact-ID match flag and forcing those hits to the top of the results. Second, it adds parent-hierarchy cycle detection to the validation suite using a Tarjan SCC algorithm, supported by a new --parent-cycle-severity configuration flag and corresponding remediation suggestions. The review feedback suggests a minor optimization in the search command implementation to avoid a redundant array copy before sorting.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // full-ID match always precedes a short-ID match for the same query, then | ||
| // offset the whole band above every remaining hit. ids are unique, so two | ||
| // exact-ID hits can never tie on score — sorting on score alone is total. | ||
| const orderedExactHits = [...exactIdHits].sort((left, right) => right.score - left.score); |
There was a problem hiding this comment.
The exactIdHits array is already a newly allocated array returned by keywordHits.filter(...) on line 1561. Therefore, creating a shallow copy using the spread operator [...exactIdHits] before calling .sort() is redundant. You can safely sort exactIdHits in-place to avoid the unnecessary array allocation.
| const orderedExactHits = [...exactIdHits].sort((left, right) => right.score - left.score); | |
| const orderedExactHits = exactIdHits.sort((left, right) => right.score - left.score); |
…ID hits Gemini review: exactIdHits is already a fresh array returned by keywordHits.filter(...), so the [...exactIdHits] spread before .sort() is an unnecessary allocation — sort in place. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JDymqzh8VsuJzFqtit5PD
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces two core read and validate integrity features: guaranteeing that exact-ID search matches rank first across all search modes (including hybrid and semantic) by bypassing score normalization and thresholding, and implementing parent-hierarchy cycle detection in pm validate using a Tarjan SCC-based cycle check. It adds the --parent-cycle-severity flag to configure policy severity, registers shell completions, defines remediation steps, and includes comprehensive unit and integration tests for both features. No review comments were provided, and the changes are well-implemented with full test coverage.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Closes the core read/validate integrity bundle (GH-281 exact-ID search rank guarantee across modes, GH-280 parent-hierarchy cycle detection) shipped in PR #282 (daada34), and regenerates CHANGELOG.md from the newly-closed items via pm-changelog 2026.6.14. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019JDymqzh8VsuJzFqtit5PD
Summary
Two agent-trust correctness fixes for pm's core read and validate primitives, both surfaced from dogfooding the large (1463-item) 2d-bots tracker. Bundled because both determine whether agents can trust pm's primitives (project philosophy: project management = context management). Tracked under Feature pm-rkie (children pm-oqgf, pm-8vul).
GH-281 — exact-ID search rank guarantee in every mode (
pm-oqgf)pm search <exact-id>already ranks the exact match #1 in keyword mode (EXACT_ID_MATCH_SCORE=1000/SHORT_ID_MATCH_SCORE=900, covered by an existing test). The residual gap was hybrid/semantic mode:combineHybridHits()normalizes keyword scores to[0,1]and blendssemantic*w + keyword*(1-w), so at the defaulthybrid_semantic_weight=0.7the exact-ID keyword boost caps at0.3and a high-semantic body-mention item can out-rank the exact target; in pure semantic mode an exact-ID item with no vector hit vanished entirely.exact_id_matchflag onSearchHit(set on the exact full/short-ID early-return inscoreDocument()).forceExactIdHitsToTop()reattaches exact-ID hits in a reserved band above every blended hit (full-ID above short-ID), applied at both the semantic and hybrid return points (after rerank), plus a score-threshold exemption — so the exact target is always#1and never dropped, in every mode.GH-280 — parent-hierarchy cycle detection in
pm validate(pm-8vul)pm validateran Tarjan SCC only over the dependency graph (blocked_by/definition_of_ready) and never traversed theparentcomposition graph, so a parent cycle (A<->B, or any-length ring) reporteddependency_cycle_count: 0with no warning while silently breakingpm list --tree, orphan detection, and rollups.buildLifecycleParentGraph(child→parent over all items — a parent cycle among closed items is still corruption) +detectLifecycleParentCycles, reusing the genericfindLifecycleDependencyCycleComponentsTarjan helper.parent_cycle_*diagnostics into the lifecycle check, emitsvalidate_hierarchy_parent_cycle[_error]warnings, adds--parent-cycle-severity off|warn|error(defaultwarn, mirrors dependency-cycle policy) with full contract/help/completion wiring, and a remediation entry (pm update <id> --parent <other-id>).Verification
pnpm typecheckclean; full suite 4536 tests pass; coverage 100/100/100/100 (29079 stmts, 22771 branches);quality:static+contracts:checkgreen.A<->Bcycle →validate_hierarchy_parent_cycle:1(warn,ok:true) /_error:1(error,ok:false, exit 1), sample pathA->B->A;offsuppresses; normal hierarchy = 0 false positives; fix-hint surfaces the remediation; exact-ID search still scores 1000 at#1.Closes #281, closes #280.
🤖 Generated with Claude Code