Skip to content

Replace the pm shell-out with the in-process SDK context engine and add --explain - #37

Merged
unbraind merged 4 commits into
mainfrom
feat/sdk-query-context-engine
Jul 26, 2026
Merged

Replace the pm shell-out with the in-process SDK context engine and add --explain#37
unbraind merged 4 commits into
mainfrom
feat/sdk-query-context-engine

Conversation

@unbraind

@unbraind unbraind commented Jul 26, 2026

Copy link
Copy Markdown
Owner

What this changes

pm-context now reads, ranks, packs, explains, and records workspace context through pm-cli's typed in-process SDK rather than parsing a pm list-all --json subprocess. The implementation uses the installed 2026.7.26 SDK as the single query/relevance/context-usage authority.

SDK behavior

  • Replaces the list shell-out with list({ full: true, includeBody: true, excludeTerminal: false }, { pmRoot }), preserving the old list-all --include-body truncation semantics.
  • Uses pm's relevance and projection-degradation packer for normal packs and handoffs.
  • Enforces the documented --max-items count ceiling after SDK token optimization.
  • Runs --explain through the exact normal filter, relevance, neighborhood, token-budget, and count-ceiling path, then explains only the emitted membership.
  • Records every emitted focus and neighborhood item as included: true; --explain remains non-mutating.
  • Reads author usage affinity and writes serving events through the canonical SDK store only.

Review hardening

All four CodeRabbit findings from the full review were reproduced and fixed. A real SDK-host integration test initializes a throwaway pm tracker, creates dependency neighbors through the typed SDK, and proves:

  • normal and explain membership are identical under --max-items;
  • --max-items 1 emits exactly one item;
  • explain does not write the usage ledger;
  • normal output records every visible focus/neighbor as included with contiguous ranks.

README wording now documents the in-process dependency and intentional best-effort serving side effect. Literal NUL bytes in TypeScript source were replaced by behavior-identical \u0000 escapes so repository tooling treats every source file as text.

PM storage recovery

pm validate found a previously committed item that pm 2026.7.26 could no longer decode because the canonical TOON encoder emits a quoted notes[] ... key:value scalar its decoder rejects. The smallest text-only recovery was followed by an audited pm history-repair; verification and storage integrity are green again. Core round-trip and recovery-path evidence is upstream in pm-cli#747.

Verification

Gate Result
TypeScript typecheck/build pass
Tests 78/78, including real tracker + SDK host integration
npm production audit 0 vulnerabilities
npm pack dry-run pass
pm-changelog check up to date
pm validate storage, dependency, history, format, and merge fence checks pass
Current honest c8 baseline 89.34 statements / 81.14 branches / 90.62 functions / 89.34 lines

The required 100/100/100/100 source-coverage gate is explicitly tracked rather than falsely claimed.

PM items

Additional SDK issues discovered during ecosystem adoption: pm-cli#738, pm-cli#740, and pm-cli#747.

pm-context read its items by spawning `pm list-all --json` and parsing the
result. That paid a process spawn per invocation and, worse, meant the package
hand-rolled relevance scoring that pm-cli already ships and tests.

pm-cli 2026.7.26 exposes a real context engine at @unbrained/pm-cli/sdk/query,
which had zero adopters across the fleet. This change:

- swaps the spawnSync `pm list-all` shell-out for in-process `list()` from
  @unbrained/pm-cli/sdk/core. Verified equivalent before switching: 24/24 item
  ids match and the item JSON is equal to the previous shell-out output.
- moves relevance and packing onto the SDK's own model rather than a local
  approximation (scoreContextCandidates / packContextCandidates).
- wires context-usage.ts to the SDK context-usage store
  (recordContextUsageServing / readContextUsageAffinity) instead of hand-parsing
  runtime/context-usage.jsonl, which pm owns.
- adds `pm context-pack --explain`, backed by evaluateContextRanking, so an
  agent can see WHY each item was selected instead of trusting an opaque pack.
  Output names the model and every contributing signal.

One type fix on top of the above: the local PmItem.parent was `string | undefined`
while the SDK's ListResultItem.parent is `string | null | undefined`. Widened the
local declaration to match what the SDK actually returns rather than casting the
difference away.

Verified: typecheck, build, 77/77 tests, changelog:check. Functional proof in a
throwaway workspace — the built extension installed into a real pm project and
`pm context-pack --status in_progress --explain` returned real per-signal scores
(model default-weighted-v1, 9 signals).

Refs: pm-context-lk82

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @unbraind, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@unbraind, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

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, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ff00f1b4-5674-411d-a28e-c49bdd10563d

📥 Commits

Reviewing files that changed from the base of the PR and between 6fdd809 and 8e5239c.

⛔ Files ignored due to path filters (8)
  • dist/context-usage.d.ts is excluded by !**/dist/**
  • dist/context-usage.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/context-usage.js is excluded by !**/dist/**
  • dist/context-usage.js.map is excluded by !**/dist/**, !**/*.map
  • dist/index.d.ts is excluded by !**/dist/**
  • dist/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (11)
  • .agents/pm/chores/pm-context-4apx.toon
  • .agents/pm/features/pm-context-lk82.toon
  • .agents/pm/history/pm-context-3s5f.jsonl
  • .agents/pm/history/pm-context-4apx.jsonl
  • .agents/pm/history/pm-context-lk82.jsonl
  • .agents/pm/issues/pm-context-3s5f.toon
  • CHANGELOG.md
  • README.md
  • context-usage.ts
  • index.ts
  • test/context-pack.test.ts

Walkthrough

pm context-pack and context-handoff now use in-process SDK queries, relevance ranking, and token-budgeted packing. A new --explain mode reports selected items without recording usage. Context usage reports include SDK-derived author affinity, with integration tests and documentation updated.

Changes

SDK Context Pipeline

Layer / File(s) Summary
SDK ranking, packing, and command flow
index.ts
Replaces subprocess item loading with SDK queries, adds relevance ranking, token-budgeted packing, --explain, serving records, and SDK affinity integration.
Usage affinity reporting
context-usage.ts
Adds SDK affinity report data, consistent touch-key encoding, and sorted Markdown rendering.
Integration verification and user-facing contracts
test/context-pack.test.ts, README.md, CHANGELOG.md
Verifies explain membership, max-items parity, ledger behavior, and documents the new context engine and flag.
Feature and coverage records
.agents/pm/features/*, .agents/pm/history/*, .agents/pm/issues/*, .agents/pm/chores/*
Records migration details, verification evidence, coverage requirements, and repaired history wording.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the main refactor and the new --explain flag.
Description check ✅ Passed The description accurately matches the PR’s in-process SDK migration and explain/usage-tracking changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/sdk-query-context-engine

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@unbraind

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@sourcery-ai review

Focus areas for this round:

  • Equivalence of the shell-out replacement. readPmItems moved from spawnSync("pm list-all --json --include-body") to list({full,includeBody,noTruncate,excludeTerminal:false},{pmRoot}). I verified 24/24 ids match and item JSON is equal on one workspace, but please sanity-check the flag→option mapping itself: is noTruncate really the equivalent of the CLI's --no-truncate, and does excludeTerminal:false correspond to list-all rather than list? A silent semantic drift here would change which items land in every context pack.
  • The parent type widening. Local PmItem.parent is now string | null | undefined to match the SDK. Nothing currently reads .parent, so this is inert today — but confirm I have not made a null leak into anything that treats absence and null differently.
  • Token-budget behaviour. Packing moved to the SDK's packContextCandidates. The budget arithmetic (TOKENS_PER_CHAR, TOKENS_PER_ITEM_SLOT) is still local. Please check the two are consistent and that --explain scores exactly the items that would be packed, not a different candidate set.
  • context-usage store. This file is owned by pm itself; we now write through recordContextUsageServing rather than appending JSONL by hand. Worth confirming we are not double-writing.

Note: greptile is unavailable for this round — greptile review returns free_reviews_limit_reached.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== PR diff stat ==='
git diff --stat HEAD^ HEAD
echo '=== Changed files ==='
git diff --name-only HEAD^ HEAD

echo '=== Relevant symbol references ==='
rg -n -C 4 \
  -e '\breadPmItems\s*\(' \
  -e '\bPmItem\b' \
  -e '\.parent\b' \
  -e '\bscoreContextCandidates\s*\(' \
  -e '\bpackContextCandidates\s*\(' \
  -e '\bevaluateContextRanking\s*\(' \
  -e '\brecordContextUsageServing\s*\(' \
  -e '\breadContextUsageAffinity\s*\(' \
  -e 'context-usage\.jsonl' \
  -e '\bspawnSync\s*\(' \
  .

echo '=== Outlines for changed TS/JS source files ==='
git diff --name-only HEAD^ HEAD | while IFS= read -r f; do
  case "$f" in
    *.ts|*.tsx|*.js|*.mjs|*.cjs)
      echo "--- $f ---"
      ast-grep outline "$f" --items all || true
      ;;
  esac
done

echo '=== SDK package files / declarations (if present) ==='
fd -HI -t f . node_modules 2>/dev/null | rg '/@unbrained/pm-cli/.*\.(d\.ts|ts|js|mjs|cjs|json)$' | head -n 200 || true

Length of output: 55749


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Package manifests and locked pm-cli version ==='
fd -HI -t f 'package.json' -o -t f 'package-lock.json' -o -t f 'pnpm-lock.yaml' -o -t f 'yarn.lock' -o -t f 'bun.lockb' . \
  | while IFS= read -r f; do
      echo "--- $f ---"
      rg -n -C 3 '`@unbrained/pm-cli`' "$f" || true
    done

echo '=== Source diff: index.ts and context-usage.ts ==='
git diff --unified=80 HEAD^ HEAD -- index.ts context-usage.ts

echo '=== Exact command paths / read / packing / explain / recording ==='
sed -n '1,90p' index.ts
sed -n '820,1085p' index.ts
sed -n '1100,1295p' index.ts
sed -n '1335,1435p' index.ts

echo '=== All direct ledger write APIs and append/write calls ==='
rg -n -C 5 \
  -e '\bappendFile(?:Sync)?\s*\(' \
  -e '\bwriteFile(?:Sync)?\s*\(' \
  -e '\brecordContextUsageServing\s*\(' \
  -e 'context-usage\.jsonl' \
  --glob '!dist/**' \
  .

echo '=== Existing tests and test references for the changed behavior ==='
fd -HI -t f . | rg '(^|/)(test|tests|__tests__|spec)(/|$)|\.(test|spec)\.[cm]?[jt]sx?$' | head -n 250
rg -n -C 3 \
  -e '\breadPmItems\s*\(' \
  -e '\bcreateSdkPacker\s*\(' \
  -e '\bbuildContextExplain\s*\(' \
  -e '\bcontext-pack\b' \
  -e '\bcontext-usage\b' \
  --glob '!dist/**' \
  .

Length of output: 1724


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Package manifests and locked pm-cli version ==='
{
  fd -HI -t f '^package\.json$' . || true
  fd -HI -t f '^package-lock\.json$' . || true
  fd -HI -t f '^pnpm-lock\.yaml$' . || true
  fd -HI -t f '^yarn\.lock$' . || true
  fd -HI -t f '^bun\.lockb$' . || true
} | sort -u | while IFS= read -r f; do
  echo "--- $f ---"
  rg -n -C 3 '`@unbrained/pm-cli`' "$f" || true
done

echo '=== Source diff: index.ts and context-usage.ts ==='
git diff --unified=80 HEAD^ HEAD -- index.ts context-usage.ts

echo '=== Exact command paths / read / packing / explain / recording ==='
sed -n '1,90p' index.ts
sed -n '820,1085p' index.ts
sed -n '1100,1295p' index.ts
sed -n '1335,1435p' index.ts

echo '=== All direct ledger write APIs and append/write calls ==='
rg -n -C 5 \
  -e '\bappendFile(?:Sync)?\s*\(' \
  -e '\bwriteFile(?:Sync)?\s*\(' \
  -e '\brecordContextUsageServing\s*\(' \
  -e 'context-usage\.jsonl' \
  --glob '!dist/**' \
  .

echo '=== Existing tests and test references for the changed behavior ==='
fd -HI -t f . | rg '(^|/)(test|tests|__tests__|spec)(/|$)|\.(test|spec)\.[cm]?[jt]sx?$' | head -n 250 || true
rg -n -C 3 \
  -e '\breadPmItems\s*\(' \
  -e '\bcreateSdkPacker\s*\(' \
  -e '\bbuildContextExplain\s*\(' \
  -e '\bcontext-pack\b' \
  -e '\bcontext-usage\b' \
  --glob '!dist/**' \
  .

Length of output: 133750


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo='unbraind/pm-cli'
tag='2026.7.26'

echo '=== Resolve release tag / commit ==='
gh api "repos/${repo}/git/ref/tags/${tag}" --jq '.object.type + " " + .object.sha' \
  || gh api "repos/${repo}/commits/main" --jq '.sha'

echo '=== Locate SDK and CLI source paths ==='
gh api "repos/${repo}/git/trees/main?recursive=1" --paginate --jq \
  '.tree[] | select(.type == "blob") | .path' \
  | rg -i '(^|/)(sdk|core|query|list|context).*\.(ts|tsx|js|mjs|d\.ts)$|(^|/)package\.json$' \
  | head -n 400

echo '=== npm package metadata and exported files ==='
npm view "`@unbrained/pm-cli`@${tag}" version dist.tarball files --json || true

echo '=== Download and inspect the published package without executing repository code ==='
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
tarball="$(npm view "`@unbrained/pm-cli`@${tag}" dist.tarball)"
curl --fail --silent --show-error --location "$tarball" --output "$tmp/pm-cli.tgz"
tar -tzf "$tmp/pm-cli.tgz" \
  | rg -i '(sdk/(core|query)|list|context).*\.(js|mjs|cjs|d\.ts)$|package/package\.json$' \
  | head -n 400
tar -xzf "$tmp/pm-cli.tgz" -C "$tmp"
rg -n -C 8 \
  -e 'function list\b' \
  -e 'const list\b' \
  -e 'excludeTerminal' \
  -e 'noTruncate' \
  -e 'packContextCandidates' \
  -e 'recordContextUsageServing' \
  -e 'type ContextPackingCandidate' \
  -e 'interface ContextPackingCandidate' \
  -e 'type ListOptions' \
  -e 'interface ListOptions' \
  "$tmp/package" \
  | head -n 1200

Length of output: 201998


🏁 Script executed:

#!/bin/bash
set -euo pipefail

tag='2026.7.26'
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
tarball="$(npm view "`@unbrained/pm-cli`@${tag}" dist.tarball)"
curl --fail --silent --show-error --location "$tarball" --output "$tmp/pm-cli.tgz"
tar -xzf "$tmp/pm-cli.tgz" -C "$tmp"

echo '=== Published SDK entry points ==='
find "$tmp/package" -type f \( -name '*.d.ts' -o -name '*.js' -o -name '*.mjs' \) \
  | grep -E '/(sdk|query)(/|[-.])' \
  | sed "s#^$tmp/package/##" \
  | head -n 250

echo '=== Relevant SDK contracts (declaration files only) ==='
find "$tmp/package" -type f -name '*.d.ts' -print0 \
  | xargs -0 rg -n -C 6 \
      -e 'interface ContextUsageServingRow' \
      -e 'type ContextUsageServingRow' \
      -e 'recordContextUsageServing' \
      -e 'interface ContextPackingCandidate' \
      -e 'type ContextPackingCandidate' \
      -e 'packContextCandidates' \
      -e 'interface ListResult' \
      -e 'type ListResult' \
      -e 'excludeTerminal' \
      -e 'noTruncate' \
  | head -n 800

echo '=== Relevant SDK implementation snippets (bounded per match) ==='
find "$tmp/package" -type f \( -name '*.js' -o -name '*.mjs' \) -print0 \
  | xargs -0 rg -l \
      -e 'recordContextUsageServing' \
      -e 'packContextCandidates' \
      -e 'excludeTerminal' \
      -e 'noTruncate' \
  | while IFS= read -r f; do
      echo "--- ${f#$tmp/package/} ---"
      rg -n -m 8 -C 3 \
        -e 'recordContextUsageServing' \
        -e 'packContextCandidates' \
        -e 'excludeTerminal' \
        -e 'noTruncate' \
        "$f" || true
    done | head -n 1000

echo '=== Local test coverage around new SDK behavior ==='
rg -n -C 5 \
  -e 'readPmItems' \
  -e 'noTruncate' \
  -e 'excludeTerminal' \
  -e 'createSdkPacker' \
  -e 'buildContextExplain' \
  -e 'included' \
  -e 'recordContextUsageServing' \
  test index.ts

echo '=== Final line-number anchors ==='
nl -ba index.ts | sed -n '835,855p;990,1028p;1130,1182p;1238,1282p;1355,1385p'
nl -ba context-usage.ts | sed -n '74,100p;268,350p'

Length of output: 10218


@unbraind Full review completed. I confirmed that excludeTerminal: false is the correct in-process equivalent of list-all, and the parent widening is inert: this package has no .parent consumer. I found the following issues:

  1. High — --explain does not explain the set that the normal command packs.
    At index.ts Line 1246, the explain branch scores only selectFocusItems() and returns. It never expands dependency neighbors or calls the createSdkPacker() path used by the normal command. Thus --without-neighborhood aside, normal packs include visible neighbors that have no explanation; with --max-items, the explain output can also contain candidates rejected by the token packer. Build the normal candidate/pack plan first, then render explanations for its included candidates (or explicitly distinguish ranked-but-excluded candidates).

  2. High — Visible neighbors are recorded as not included.
    index.ts Lines 1166-1168 creates ledger rows with included: true for focus items but included: false for every neighbor. Those neighbors are nevertheless rendered into the context pack/handoff, so context-usage will treat shown context as unserved: it will omit it from conversion/waste metrics and from future affinity feedback. Mark every item actually emitted in the pack as included, or record the complete ranked candidate list separately and set included based on final pack membership.

  3. Medium — noTruncate: true broadens the original shell-out’s behavior.
    index.ts Line 845 adds noTruncate: true, but the replaced command was pm list-all --json --include-body without --no-truncate. In pm-cli 2026.7.26, list-all maps to list with excludeTerminal: false; noTruncate independently overrides the normal list limit. The tested 24-item workspace cannot expose this difference, but a larger workspace can now feed additional items into ranking and packing. Remove noTruncate to preserve the original command exactly, or add --no-truncate to the old-equivalence baseline and document the intentional corpus expansion.

  4. Low — README behavior description is now stale.
    README Line 30 says the extension is read-only and shells out to the active pm binary. Normal context-pack and context-handoff now use the in-process SDK and write serving events through recordContextUsageServing at index.ts Line 1172. Please update that description so users understand the dependency and intentional ledger side effect.

No duplicate hand-written ledger write remains: the production extension has a single recordContextUsageServing path per normal pack/handoff invocation, and --explain returns before recording.

✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 31 minutes.

Run --explain through the same relevance, neighborhood, token-packing, and max-item path as normal context packs. Record every emitted neighbor as included in context-usage while keeping explain non-mutating and preserving legacy list truncation semantics.

Add a real SDK-host integration test over a throwaway pm tracker, document the in-process query and serving contract, enforce the max-items ceiling after SDK token optimization, and replace literal NUL source bytes with portable escapes.

Repair the previously unreadable pm-context-4apx TOON item with an audited history re-anchor, track the upstream pm-cli round-trip defect as unbraind/pm-cli#747, and record the honest 100/100/100/100 coverage gap as pm-context-3s5f.
@unbraind

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

/gemini review

@sourcery-ai review

Post-fix review requested for commit c284909. All four prior CodeRabbit findings are addressed, with a real SDK-host integration test covering exact packed/explained membership, max-items enforcement, explain non-mutation, and included neighbor serving rows. Please also inspect the audited TOON tracker recovery and the max-items post-token-budget ceiling. Greptile remains unavailable on the free review limit.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

@unbraind I’ll perform a post-fix full review of commit c284909, with particular attention to the SDK-host integration coverage, TOON tracker recovery, and enforcing max-items after token-budget packing.

✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 24 minutes.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging the Gemini sunset notice at #37 (comment). /gemini review was re-triggered after commit c284909 as requested by repository workflow, but this bot is unavailable rather than silently treated as a successful review.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging the earlier CodeRabbit rate-limit response at #37 (comment). A later full review did complete and produced four actionable findings; all four are fixed in c284909, and a fresh full review has been requested.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@unbraind

Copy link
Copy Markdown
Owner Author

Resolved every finding in the completed CodeRabbit review at #37 (comment): explain now uses exact final pack membership; every emitted neighbor is recorded included; noTruncate was removed; README documents SDK reads and serving writes. The post-review audit also enforces the --max-items count ceiling after SDK token optimization. A real tracker integration test proves membership parity, count enforcement, non-mutating explain, and ledger rows.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging Sourcery's rate-limited review on commit 00c5d22. No findings were returned. Review was re-requested for c284909; its availability will be reported as neutral if the service remains rate-limited, not as substantive approval.

Resolve both post-push DeepScan findings without changing runtime behavior, and record the remediation in pm-context-lk82.
@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging the first post-fix Gemini sunset response at #37 (comment). It contains no code finding; the service is unavailable.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging CodeRabbit's post-fix response at #37 (comment). It returned no new finding but was rate-limited. Since c66c83f follows, a new full review is requested below as required.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging the second Gemini sunset response at #37 (comment). It contains no code finding; this duplicate service notice is recorded and reacted to.

@unbraind

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

/gemini review

@sourcery-ai review

Final post-DeepScan review requested for c66c83f. This follow-up only removes the two unused imports reported by DeepScan and records the remediation in the linked pm item. Full release:check remains green at 78/78 tests and zero production audit findings.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

@unbraind I’ll perform a final full review of the c66c83f follow-up, including the unused-import cleanup and linked pm-item remediation.

✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 19 minutes.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging the final-round Gemini sunset response at #37 (comment). No review occurred because the consumer service is retired.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging CodeRabbit's final-round response at #37 (comment). It returned no finding but reports review capacity in 19 minutes. All executable checks are green; I will wait within the requested 20-minute window and request one final substantive review when capacity opens.

@unbraind

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

The stated cooldown has elapsed. Please perform the final substantive review of c66c83f now. All prior findings and both DeepScan findings are resolved; CI, Semgrep, DeepScan, and Hound are green.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

@unbraind I’m initiating a final substantive full review of commit c66c83f, including verification of the prior fixes and the final cleanup.

✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 2 seconds.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledging CodeRabbit final-review initiation at #37 (comment). The check is now genuinely pending; I will wait for the substantive result and address any finding before merge.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 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.

Inline comments:
In @.agents/pm/features/pm-context-lk82.toon:
- Line 3: The feature record still documents superseded SDK APIs and the removed
noTruncate parity behavior. Update its description and line 16 to match the
shipped implementation, using rankContextItems/createSdkPacker,
buildContextExplain/scoreContextItems, and the legacy truncation behavior;
alternatively, clearly mark the earlier API names and noTruncate note as
superseded intermediate details.

In `@index.ts`:
- Around line 1045-1061: Clarify the explain output contract around
buildContextExplain: state that ranks and scores are computed from the supplied
packed subset, not the full corpus or workspace. Add this clarification to the
generated report metadata or the --explain flag description, ensuring readers do
not interpret rank 1 as the workspace-wide top result.
- Around line 1228-1231: Guard the full-corpus ranking in the flow around
rankContextItems and createSdkPacker so rankContextItems runs only when maxItems
is set. Initialize packer as undefined for the no-limit path, and preserve
passing rankedAll to createSdkPacker when maxItems is enabled. Apply the same
conditional change to the corresponding context-handoff logic near the second
occurrence.
- Around line 938-952: Update scoreContextItems to build a single byId Map from
item IDs to items at its start, then use that index when resolving candidates
instead of repeatedly calling byIdOrFail with items.find. Preserve the existing
descriptive CommandError behavior for missing IDs, either through the indexed
lookup helper or equivalent validation.
- Around line 1109-1132: Update resolveSdkRankOptions to handle readSettings or
resolveRuntimeStatusRegistry failures by falling back to the built-in runtime
status registry instead of throwing CommandError, preserving ranking for
malformed settings. Remove the unused _items parameter and update both call
sites to invoke resolveSdkRankOptions without that argument.

In `@README.md`:
- Around line 30-34: Refresh the README “Max items” section to describe
createSdkPacker’s token-budgeted selection, including projection degradation and
--max-items as an additional item-count ceiling. Remove the outdated explanation
that focus items take priority and neighbors are trimmed solely to fit the
remaining count budget.

In `@test/context-pack.test.ts`:
- Around line 602-607: In the context-pack runner invocation, add a brief
comment explaining that omitting author is intentional because it preserves the
ledger state for the final serving assertions. Keep the command options and
expected ledger behavior unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ce81a862-9ff9-4898-8b11-683605fbad0a

📥 Commits

Reviewing files that changed from the base of the PR and between 6fdd809 and c66c83f.

⛔ Files ignored due to path filters (8)
  • dist/context-usage.d.ts is excluded by !**/dist/**
  • dist/context-usage.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/context-usage.js is excluded by !**/dist/**
  • dist/context-usage.js.map is excluded by !**/dist/**, !**/*.map
  • dist/index.d.ts is excluded by !**/dist/**
  • dist/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (11)
  • .agents/pm/chores/pm-context-4apx.toon
  • .agents/pm/features/pm-context-lk82.toon
  • .agents/pm/history/pm-context-3s5f.jsonl
  • .agents/pm/history/pm-context-4apx.jsonl
  • .agents/pm/history/pm-context-lk82.jsonl
  • .agents/pm/issues/pm-context-3s5f.toon
  • CHANGELOG.md
  • README.md
  • context-usage.ts
  • index.ts
  • test/context-pack.test.ts

Comment thread .agents/pm/features/pm-context-lk82.toon Outdated
Comment thread index.ts
Comment thread index.ts
Comment thread index.ts Outdated
Comment thread index.ts Outdated
Comment thread README.md
Comment thread test/context-pack.test.ts
Index relevance candidates once, avoid unnecessary full-corpus ranking, and make emitted-pack-relative explain scores explicit in the API, CLI, docs, and real tracker integration test. Refresh the honest coverage baseline and preserve the remaining 100% gate as tracked work.
@unbraind

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

Please re-review commit 8e5239c and confirm the seven final findings are fully resolved.

@unbraind

Copy link
Copy Markdown
Owner Author

/gemini review

Please re-review the latest commit 8e5239c.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@unbraind

Copy link
Copy Markdown
Owner Author

@sourcery-ai review

Please re-review the latest commit 8e5239c.

@unbraind

Copy link
Copy Markdown
Owner Author

@greptileai please review the latest commit 8e5239c. If this repository is not enabled, please report that explicitly.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledged the Gemini Code Assist sunset response. No Gemini findings are available for commit 8e5239c; the remaining enabled review and required CI gates continue independently.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== PR metadata and recent commits ==='
gh pr view 37 --repo unbraind/pm-context --json number,headRefOid,baseRefOid,commits \
  --jq '{number, headRefOid, baseRefOid, commits: [.commits[] | {oid, messageHeadline}]}'

echo '=== Commit 8e5239c metadata and changed files ==='
git show --no-ext-diff --format=fuller --stat 8e5239c
git diff-tree --no-commit-id --name-status -r 8e5239c

echo '=== Commit diff ==='
git show --no-ext-diff --format= --unified=80 8e5239c

echo '=== Recent PR review/comment bodies (to identify the seven final findings) ==='
gh api --paginate 'repos/unbraind/pm-context/issues/37/comments?per_page=100' \
  --jq '.[] | select(.user.login == "coderabbitai[bot]" or .user.login == "coderabbitai") | {created_at, body}' \
  | tail -n 80
gh api --paginate 'repos/unbraind/pm-context/pulls/37/comments?per_page=100' \
  --jq '.[] | select(.user.login == "coderabbitai[bot]" or .user.login == "coderabbitai") | {created_at, path, line, body}' \
  | tail -n 100

echo '=== Outline and focused symbols in changed TypeScript files ==='
git diff-tree --no-commit-id --name-only -r 8e5239c \
  | while IFS= read -r f; do
      case "$f" in
        *.ts|*.tsx|*.js|*.mjs|*.cjs)
          echo "--- outline: $f ---"
          ast-grep outline "$f" --items all || true
          ;;
      esac
    done

echo '=== Focused implementation and test references ==='
rg -n -C 5 \
  -e '\breadPmItems\s*\(' \
  -e '\bcreateSdkPacker\s*\(' \
  -e '\bbuildContextExplain\s*\(' \
  -e '\brecordPackServing\s*\(' \
  -e '\brecordContextUsageServing\s*\(' \
  -e '\bmaxItems\b' \
  -e '\bincluded\s*:' \
  -e '\bnoTruncate\b' \
  -e '\bexcludeTerminal\b' \
  index.ts context-usage.ts test 2>/dev/null || true

Length of output: 202078


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Source-only files changed by 8e5239c ==='
git diff-tree --no-commit-id --name-only -r 8e5239c \
  | rg -v '^(dist/|.*\.map$)' \
  | sort

echo '=== Exact feature-record text ==='
sed -n '1,120p' .agents/pm/features/pm-context-lk82.toon

echo '=== README context-engine and max-items sections ==='
rg -n -C 6 -e 'SDK' -e 'shell' -e '[Mm]ax items' -e 'projection' README.md

echo '=== Final implementation regions ==='
nl -ba index.ts | sed -n '827,855p;920,963p;985,1108p;1110,1170p;1218,1270p;1346,1376p'
nl -ba test/context-pack.test.ts | sed -n '558,652p'

echo '=== Obsolete implementation/documentation terms in non-generated files ==='
rg -n -C 2 \
  -e '\bscoreContextCandidates\b' \
  -e '\bpackContextCandidates\b' \
  -e '\bevaluateContextRanking\b' \
  -e '\bnoTruncate\b' \
  -e 'spawnSync\("pm"' \
  --glob '!dist/**' \
  --glob '!*.map' \
  .

echo '=== Diff limited to the seven reviewed source/doc files ==='
git diff --no-ext-diff --check 8e5239c^ 8e5239c -- \
  .agents/pm/features/pm-context-lk82.toon README.md index.ts test/context-pack.test.ts

git diff --no-ext-diff --unified=12 8e5239c^ 8e5239c -- \
  .agents/pm/features/pm-context-lk82.toon README.md index.ts test/context-pack.test.ts

Length of output: 9842


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Final source: list equivalence, scoring index, packing, explain ==='
sed -n '827,1108p' index.ts

echo '=== Final source: rank options, serving, context-pack and handoff command paths ==='
sed -n '1110,1170p' index.ts
sed -n '1218,1270p' index.ts
sed -n '1346,1376p' index.ts

echo '=== Final integration test ==='
sed -n '558,652p' test/context-pack.test.ts

echo '=== Focused source search with line anchors ==='
rg -n -C 3 \
  -e 'const byId = new Map' \
  -e 'function byIdOrFail' \
  -e 'ranking_scope' \
  -e 'Ranking scope:' \
  -e 'resolveSdkRankOptions\(' \
  -e 'if \(maxItems\)' \
  -e 'Intentionally omit author' \
  index.ts test/context-pack.test.ts

echo '=== Non-generated stale-term search ==='
rg -n -C 2 \
  -e '\bscoreContextCandidates\b' \
  -e '\bevaluateContextRanking\b' \
  -e '\bnoTruncate\b' \
  -e 'read-only and shells out' \
  --glob '!dist/**' \
  --glob '!*.map' \
  . || true

echo '=== Commit hygiene ==='
git diff --no-ext-diff --check 8e5239c^ 8e5239c
git diff --no-ext-diff --unified=12 8e5239c^ 8e5239c -- \
  .agents/pm/features/pm-context-lk82.toon README.md index.ts test/context-pack.test.ts

Length of output: 55890


@unbraind Full re-review of 8e5239c complete. I confirmed all seven final findings are resolved:

  1. Feature record now reflects the shipped APIs and explicitly supersedes the earlier noTruncate note.
  2. Candidate resolution uses a prebuilt Map, removing the quadratic lookup.
  3. Explain output declares ranking_scope: "emitted_pack" in JSON, Markdown, CLI help, and README.
  4. The unused resolveSdkRankOptions parameter is removed. The proposed malformed-settings fallback was correctly withdrawn: readPmItems() already performs the canonical SDK read first and fails on unreadable settings.
  5. Full-corpus pre-ranking now runs only when --max-items requires SDK packing, in both command paths.
  6. README now accurately describes token-budgeted projection degradation plus the final hard item-count ceiling.
  7. The integration test documents why its parity probe intentionally omits author.

I also confirmed the SDK list call preserves default truncation semantics, the feature history labels its obsolete noTruncate reference as superseded, and git diff --check is clean. No additional actionable findings from this static review.

✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 48 minutes.

@unbraind

Copy link
Copy Markdown
Owner Author

Acknowledged. Thank you for confirming all seven dispositions on 8e5239c, including the malformed-settings fallback rationale, and reporting no additional actionable findings. The full local 78/78 release gate and all enabled GitHub checks are green; the explicit coverage gap remains tracked in pm-context-3s5f.

@unbraind
unbraind merged commit 6517ae8 into main Jul 26, 2026
7 checks passed
@unbraind
unbraind deleted the feat/sdk-query-context-engine branch July 26, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant