fix(mcp): route product feedback prompts to packet#131
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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 a 'product-role feedback packet' feature to provide specialized guidance and supplemental IDs when product maintenance intents are detected. Key changes include the addition of new intent signals, heuristic compiler rules, and updated projection logic in the answer builder, along with corresponding test coverage. Reviewers suggested improving maintainability by centralizing the hardcoded list of packet IDs into a shared constant and optimizing performance by passing pre-calculated intent flags to helper functions to avoid redundant string processing.
| ] as const; | ||
|
|
There was a problem hiding this comment.
The list of IDs for the product-role feedback packet is hardcoded in multiple locations. Consider defining it as a shared constant here to improve maintainability and ensure consistency across the runtime and compiler.
] as const;
export const PRODUCT_ROLE_FEEDBACK_PACKET_IDS = ['E.12', 'A.1.1', 'A.15', 'A.2.2', 'A.2.3'] as const;| PRODUCT_ROLE_FEEDBACK_OUTPUT_SIGNALS, | ||
| PRODUCT_ROLE_FEEDBACK_ROLE_SIGNALS, | ||
| } from './route-intent-signals.js'; |
There was a problem hiding this comment.
Import the newly defined PRODUCT_ROLE_FEEDBACK_PACKET_IDS constant.
| PRODUCT_ROLE_FEEDBACK_OUTPUT_SIGNALS, | |
| PRODUCT_ROLE_FEEDBACK_ROLE_SIGNALS, | |
| } from './route-intent-signals.js'; | |
| PRODUCT_ROLE_FEEDBACK_OUTPUT_SIGNALS, | |
| PRODUCT_ROLE_FEEDBACK_PACKET_IDS, | |
| PRODUCT_ROLE_FEEDBACK_ROLE_SIGNALS, | |
| } from './route-intent-signals.js'; |
| const productRoleFeedbackNodeIds = ['E.12', 'A.1.1', 'A.15', 'A.2.2', 'A.2.3'].filter( | ||
| (id) => id in patternNodes || id in routeNodes, | ||
| ); |
There was a problem hiding this comment.
Use the PRODUCT_ROLE_FEEDBACK_PACKET_IDS constant instead of hardcoding the ID list.
| const productRoleFeedbackNodeIds = ['E.12', 'A.1.1', 'A.15', 'A.2.2', 'A.2.3'].filter( | |
| (id) => id in patternNodes || id in routeNodes, | |
| ); | |
| const productRoleFeedbackNodeIds = PRODUCT_ROLE_FEEDBACK_PACKET_IDS.filter( | |
| (id) => id in patternNodes || id in routeNodes, | |
| ); |
| getPartCDraftsByCluster, | ||
| selectBestAnchors, | ||
| } from './compiler.js'; | ||
| import { hasProductRoleFeedbackIntent } from './route-intent-signals.js'; |
There was a problem hiding this comment.
Import the PRODUCT_ROLE_FEEDBACK_PACKET_IDS constant.
| import { hasProductRoleFeedbackIntent } from './route-intent-signals.js'; | |
| import { hasProductRoleFeedbackIntent, PRODUCT_ROLE_FEEDBACK_PACKET_IDS } from './route-intent-signals.js'; |
| ? ['E.12', 'A.1.1', 'A.15', 'A.2.2', 'A.2.3'].filter( | ||
| (id) => Boolean(snapshot.compiledNodes[id]), | ||
| ) |
There was a problem hiding this comment.
Use the PRODUCT_ROLE_FEEDBACK_PACKET_IDS constant instead of hardcoding the ID list.
| ? ['E.12', 'A.1.1', 'A.15', 'A.2.2', 'A.2.3'].filter( | |
| (id) => Boolean(snapshot.compiledNodes[id]), | |
| ) | |
| ? PRODUCT_ROLE_FEEDBACK_PACKET_IDS.filter( | |
| (id) => Boolean(snapshot.compiledNodes[id]), | |
| ) |
| `Acceptance check: ${routeAcceptanceCheck(routeNodeId, route, question)}.`, | ||
| `Next move: ${routeNextMove(routeNodeId, route, question)}.`, |
There was a problem hiding this comment.
To avoid redundant intent detection (which involves string normalization and multiple substring checks), pass the already calculated productRoleFeedbackPacket boolean to the helper functions instead of the raw question string.
| `Acceptance check: ${routeAcceptanceCheck(routeNodeId, route, question)}.`, | |
| `Next move: ${routeNextMove(routeNodeId, route, question)}.`, | |
| `Acceptance check: ${routeAcceptanceCheck(routeNodeId, route, productRoleFeedbackPacket)}.`, | |
| `Next move: ${routeNextMove(routeNodeId, route, productRoleFeedbackPacket)}.`, |
| function routeAcceptanceCheck( | ||
| routeNodeId: string, | ||
| route: Snapshot['routeGraph']['nodes'][string], | ||
| question: string, | ||
| ): string { | ||
| if (isProductRoleFeedbackPacket(routeNodeId, question)) { |
There was a problem hiding this comment.
Update the signature to accept the pre-calculated boolean to avoid redundant intent detection.
| function routeAcceptanceCheck( | |
| routeNodeId: string, | |
| route: Snapshot['routeGraph']['nodes'][string], | |
| question: string, | |
| ): string { | |
| if (isProductRoleFeedbackPacket(routeNodeId, question)) { | |
| function routeAcceptanceCheck( | |
| routeNodeId: string, | |
| route: Snapshot['routeGraph']['nodes'][string], | |
| isProductRoleFeedback: boolean, | |
| ): string { | |
| if (isProductRoleFeedback) { |
| function routeNextMove( | ||
| routeNodeId: string, | ||
| route: Snapshot['routeGraph']['nodes'][string], | ||
| question: string, | ||
| ): string { | ||
| if (isProductRoleFeedbackPacket(routeNodeId, question)) { |
There was a problem hiding this comment.
Update the signature to accept the pre-calculated boolean to avoid redundant intent detection.
| function routeNextMove( | |
| routeNodeId: string, | |
| route: Snapshot['routeGraph']['nodes'][string], | |
| question: string, | |
| ): string { | |
| if (isProductRoleFeedbackPacket(routeNodeId, question)) { | |
| function routeNextMove( | |
| routeNodeId: string, | |
| route: Snapshot['routeGraph']['nodes'][string], | |
| isProductRoleFeedback: boolean, | |
| ): string { | |
| if (isProductRoleFeedback) { |
There was a problem hiding this comment.
Pull request overview
Routes "product maintainer" / product-role feedback prompts to the documented packet (E.12, A.1.1, A.15, A.2.2, A.2.3) under route:project-alignment, instead of drifting into spec-authoring IDs. Implements a new heuristic seed rule, projects packet-specific IDs/acceptance/next-move guidance in the answer projector, refreshes the published snapshot/manifest fingerprint to match, and adds regression tests at query, runtime, and MCP layers.
Changes:
- New
product-role-feedbackheuristic seed rule + intent signal constants andhasProductRoleFeedbackIntentpredicate. buildRouteAnswerprojects supplemental packet IDs, packet-specific acceptance check, and next move when the intent matches onroute:project-alignment.- Refresh
published/current/manifest.jsonandsnapshot.jsonso the compiler fingerprint matches the new rule.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/runtime/route-intent-signals.ts | Adds product-role role/output signal lists and the hasProductRoleFeedbackIntent predicate. |
| src/runtime/compiler.ts | Registers the new product-role-feedback heuristic seed rule pointing at route:project-alignment. |
| src/runtime/answer-projector.ts | Projects packet IDs and packet-specific acceptance/next-move text when the intent matches. |
| published/current/manifest.json | Updates compiler fingerprint and publishedAt to reflect the new compiled rules. |
| published/current/fpf-index/snapshot.json | Adds the compiled product-role-feedback rule and refreshes fingerprint/builtAt. |
| tests/query-contracts.test.ts | Adds routing + projection regression coverage for the maintainer prompt. |
| tests/fpf-spec-runtime.test.ts | Asserts the maintainer prompt projects the packet under the live runtime. |
| tests/mcp-server.test.ts | Asserts MCP query_fpf_spec returns the packet for the maintainer prompt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const hasOutputSignal = PRODUCT_ROLE_FEEDBACK_OUTPUT_SIGNALS.some((phrase) => | ||
| normalizedQuestion.includes(phrase), | ||
| ); | ||
| return hasRoleSignal && hasOutputSignal; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0d1384800
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hasOutputSignal = PRODUCT_ROLE_FEEDBACK_OUTPUT_SIGNALS.some((phrase) => | ||
| normalizedQuestion.includes(phrase), | ||
| ); | ||
| return hasRoleSignal && hasOutputSignal; |
There was a problem hiding this comment.
The new packet projector can silently diverge from the new routing rule: buildHeuristicSeedRules matches product-role-feedback when role signals are present and either output signals or bounded-retrieval signals are present (tokenized matching), but hasProductRoleFeedbackIntent requires a raw substring output-signal match only. This means queries can route to route:project-alignment via product-role-feedback yet skip the packet IDs/acceptance/next-move projection (for example prompts using bounded-retrieval wording or hyphenated role phrasing), reintroducing the same miss this commit is trying to fix.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b0d1384. Configure here.
| rules.push({ | ||
| name: 'product-role-feedback', | ||
| allOf: [[...PRODUCT_ROLE_FEEDBACK_ROLE_SIGNALS]], | ||
| anyOf: [[...PRODUCT_ROLE_FEEDBACK_OUTPUT_SIGNALS, ...AGENT_WORKFLOW_BOUNDED_RETRIEVAL_SIGNALS]], |
There was a problem hiding this comment.
Seed rule anyOf broader than projection intent check
Low Severity
The product-role-feedback heuristic seed rule's anyOf combines both PRODUCT_ROLE_FEEDBACK_OUTPUT_SIGNALS and AGENT_WORKFLOW_BOUNDED_RETRIEVAL_SIGNALS, but hasProductRoleFeedbackIntent only checks PRODUCT_ROLE_FEEDBACK_OUTPUT_SIGNALS. A query matching a role signal plus a bounded-retrieval signal (e.g. "work packet") but no output signal will trigger the seed (boosting route:project-alignment to 92), yet isProductRoleFeedbackPacket returns false, so the product-role feedback packet IDs, constraints, acceptance check, and next-move guidance are all silently omitted.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b0d1384. Configure here.
|
Friday merge-captain status: blocked; not merging this head. Observed checks are green, and the advertised maintainer prompt returns the intended product-role feedback packet. However, the current head still has unresolved review threads on a real behavior gap: Local replay on TMPDIR="$PWD/.tmp/bun" bun run cli -- query --question "As a product maintainer, build a work packet without pasting the whole FPF." --mode compactResult: Other merge-policy blockers also remain: no trusted non-author approval exists on the current head, and the TPR fast-track path documented in this repo still requires that approval before merge/auto-merge. Validation I reran in a temp PR worktree: bun install --frozen-lockfile
TMPDIR="$PWD/.tmp/bun" bun run check
TMPDIR="$PWD/.tmp/bun" bunx rstest run tests/query-contracts.test.ts tests/fpf-spec-runtime.test.ts tests/mcp-server.test.ts
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a2fe17ee8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const hasRoleSignal = PRODUCT_ROLE_FEEDBACK_ROLE_SIGNALS.some((phrase) => | ||
| normalizedQuestion.includes(phrase), | ||
| ); |
There was a problem hiding this comment.
hasProductRoleFeedbackIntent uses raw substring checks, but route seeding for the same rule uses token-based matching in matchesSeedRule (src/runtime/candidate-seeder.ts), so equivalent phrasings can diverge. For example, a query containing hyphenated product-feedback (or non-adjacent role tokens) can still satisfy the seed rule and route to route:project-alignment, but this function returns false and skips the product-role packet projection/acceptance guidance. This reintroduces inconsistent behavior for prompts this change is meant to standardize.
Useful? React with 👍 / 👎.
|
Friday merge-captain status: blocked; not merging this head. The previous bounded-retrieval-only maintainer prompt is fixed on bun install --frozen-lockfile
TMPDIR="$PWD/.tmp/bun" bun run check
TMPDIR="$PWD/.tmp/bun" bunx rstest run tests/query-contracts.test.ts tests/fpf-spec-runtime.test.ts tests/mcp-server.test.tsObserved results: However, there is still a current-head unresolved behavior blocker matching the latest Codex review thread: seed matching is tokenized, while TMPDIR="$PWD/.tmp/bun" bun run cli -- query --question "As a product-feedback maintainer, build a work packet without pasting the whole FPF." --mode compactResult: Other merge-policy blockers also remain: no trusted non-author approval exists on the current head, and the visible Vercel status is failing with the account deployment blocked link. Please align projector intent detection with the seed-rule matching semantics and add a regression for the hyphenated/tokenized product-feedback maintainer prompt before the next merge pass. |
Source evidence
Problem
The compact maintainer prompt drifted into spec-authoring IDs like
E.17.AUDandE.8instead of the documented packet centered onE.12,A.1.1,A.15,A.2.2, andA.2.3.Change made
product-role-feedbackheuristic seed that routes maintainer/product-feedback prompts toroute:project-alignment.published/current/**against the committedpublished/current/FPF-Spec.mdsurface so the committed snapshot/compiler fingerprint matches the runtime change.Validation
TMPDIR="$PWD/.tmp/bun" bunx rstest run tests/query-contracts.test.ts tests/fpf-spec-runtime.test.ts tests/mcp-server.test.tsTMPDIR="$PWD/.tmp/bun" bun run checktsc --noEmit).TMPDIR="$PWD/.tmp/bun" bun run cli -- query --question "As a product maintainer, turn live product smoke evidence plus recurring role-feedback discussions into one adoption improvement, severity, and validation path without reading or pasting the full FPF." --mode compactstatus: "ok"withroute:project-alignment,E.12,A.1.1,A.15,A.2.2, andA.2.3.Residual risk
route:project-alignment, not as a first-class dedicated route/doc selector. If later product-role prompts diverge from this maintainer wording, they may need additional intent tuning..tmp/validation directory unstaged because shell policy blocked deleting it directly in this session.Friday handoff
Friday review/merge captain should replay the maintainer CLI/MCP prompt on the PR head, verify the refreshed published snapshot is intentional, and merge only after confirming the compact packet stays bounded to product-feedback work rather than spec-authoring guidance.
Note
Medium Risk
Adjusts query intent detection and route answer projection for
route:project-alignment, which can change which IDs/guidance are returned for some prompts; mitigated by added regression tests.Overview
Routes product-maintainer/product-feedback prompts to the documented product-role feedback packet by adding new
PRODUCT_ROLE_FEEDBACK_*intent signals and aproduct-role-feedbackheuristic seed rule that boostsroute:project-alignment.When that intent matches,
buildRouteAnswernow injects supplemental packet IDs (E.12,A.1.1,A.15,A.2.2,A.2.3) and swaps in maintainer-specific acceptance/next-move guidance plus constraints to avoid escalating to spec-writing IDs.Refreshes the published artifacts (
published/current/manifest.json+ snapshot metadata/rules) and adds regression coverage across query-contracts, runtime, and MCP server tests to assert the new routing/projection behavior.Reviewed by Cursor Bugbot for commit b0d1384. Configure here.