Reduce MCP list endpoint context#40
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reduces token/context usage for MCP list-style endpoints by adding compact field selection and title search, introducing offset pagination for tasks, and lowering default list limits.
Changes:
- Add
fieldsandsearchparameters tolist_storiesandget_epics; addsearch+offsettoget_tasks. - Lower default
limitfrom 50 → 10 across tasks/stories/epics list schemas and DB query defaults. - Implement story/epic field selection in the core DB layer and extend option types + tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ohno-mcp/src/server.ts | Extends MCP tool schemas/docs for limit defaults, offset pagination, fields selection, and title search. |
| packages/ohno-mcp/src/server.test.ts | Adds/updates schema + tool behavior tests for new params and smaller defaults. |
| packages/ohno-core/src/types.ts | Updates option types and relaxes Story fields to support minimal-field list responses. |
| packages/ohno-core/src/types.test.ts | Adds type-level tests for minimal story list shapes and new query options. |
| packages/ohno-core/src/db.ts | Implements search, pagination (tasks), and field selection for stories/epics; lowers default query limits. |
| packages/ohno-core/src/db.test.ts | Adds coverage for new query options and updates expectations around minimal defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const STORY_FIELD_SETS: Record<FieldSet, string[]> = { | ||
| minimal: ["id", "title", "status", "epic_id"], | ||
| standard: ["id", "title", "status", "epic_id", "description", "created_at", "updated_at"], | ||
| full: ["id", "epic_id", "title", "description", "status", "created_at", "updated_at"], | ||
| }; | ||
|
|
||
| const EPIC_FIELD_SETS: Record<FieldSet, string[]> = { | ||
| minimal: ["id", "title", "status", "priority"], | ||
| standard: ["id", "project_id", "title", "description", "priority", "status", "created_at", "updated_at"], | ||
| full: ["id", "project_id", "title", "description", "priority", "status", "created_at", "updated_at"], |
There was a problem hiding this comment.
STORY_FIELD_SETS and EPIC_FIELD_SETS currently make "standard" and "full" effectively identical (story differs only by column order; epic is identical). This makes the fields parameter misleading and harder to evolve. Consider making standard omit some heavier/less-needed fields (e.g., timestamps/project_id) so full is meaningfully larger, or documenting that standard == full for these tables.
| const STORY_FIELD_SETS: Record<FieldSet, string[]> = { | |
| minimal: ["id", "title", "status", "epic_id"], | |
| standard: ["id", "title", "status", "epic_id", "description", "created_at", "updated_at"], | |
| full: ["id", "epic_id", "title", "description", "status", "created_at", "updated_at"], | |
| }; | |
| const EPIC_FIELD_SETS: Record<FieldSet, string[]> = { | |
| minimal: ["id", "title", "status", "priority"], | |
| standard: ["id", "project_id", "title", "description", "priority", "status", "created_at", "updated_at"], | |
| full: ["id", "project_id", "title", "description", "priority", "status", "created_at", "updated_at"], | |
| const STORY_FULL_FIELDS = ["id", "epic_id", "title", "description", "status", "created_at", "updated_at"]; | |
| const STORY_FIELD_SETS: Record<FieldSet, string[]> = { | |
| minimal: ["id", "title", "status", "epic_id"], | |
| // `standard` intentionally aliases `full` for stories today to preserve existing API behavior. | |
| // `minimal` is the only reduced projection until a smaller story-standard shape is introduced. | |
| standard: STORY_FULL_FIELDS, | |
| full: STORY_FULL_FIELDS, | |
| }; | |
| const EPIC_FULL_FIELDS = ["id", "project_id", "title", "description", "priority", "status", "created_at", "updated_at"]; | |
| const EPIC_FIELD_SETS: Record<FieldSet, string[]> = { | |
| minimal: ["id", "title", "status", "priority"], | |
| // `standard` intentionally aliases `full` for epics today to preserve existing API behavior. | |
| // `minimal` is the only reduced projection until a smaller epic-standard shape is introduced. | |
| standard: EPIC_FULL_FIELDS, | |
| full: EPIC_FULL_FIELDS, |
| limit: { type: "number", description: "Maximum stories to return (1-100)", default: 50 }, | ||
| limit: { type: "number", description: "Maximum stories to return (1-100)", default: 10 }, | ||
| offset: { type: "number", description: "Number of stories to skip", default: 0 }, | ||
| fields: { type: "string", enum: ["minimal", "standard", "full"], description: "Field set to return: minimal (default, no descriptions), standard (with descriptions), full (all fields)", default: "minimal" }, |
There was a problem hiding this comment.
The fields description implies full returns more data than standard, but in the current DB implementation for stories the field sets are effectively the same (and standard already includes timestamps). Either make full include additional fields beyond standard, or adjust the tool schema description so clients don’t assume full provides extra data.
| fields: { type: "string", enum: ["minimal", "standard", "full"], description: "Field set to return: minimal (default, no descriptions), standard (with descriptions), full (all fields)", default: "minimal" }, | |
| fields: { type: "string", enum: ["minimal", "standard", "full"], description: "Field set to return: minimal (default, no descriptions), standard (includes descriptions and timestamps), full (same as standard)", default: "minimal" }, |
| priority: { type: "string", enum: ["P0", "P1", "P2", "P3"], description: "Filter by priority" }, | ||
| limit: { type: "number", description: "Maximum epics to return (1-100)", default: 50 }, | ||
| limit: { type: "number", description: "Maximum epics to return (1-100)", default: 10 }, | ||
| fields: { type: "string", enum: ["minimal", "standard", "full"], description: "Field set to return: minimal (default, no descriptions), standard (with descriptions), full (all fields)", default: "minimal" }, |
There was a problem hiding this comment.
Same issue as list_stories: the tool schema describes full as “all fields”, but for epics the DB field sets for standard and full are identical today. Either differentiate the field sets (so full is truly larger) or update the description to avoid misleading callers.
| fields: { type: "string", enum: ["minimal", "standard", "full"], description: "Field set to return: minimal (default, no descriptions), standard (with descriptions), full (all fields)", default: "minimal" }, | |
| fields: { type: "string", enum: ["minimal", "standard", "full"], description: "Field set to return: minimal (default, no descriptions), standard (with descriptions), full (currently same as standard)", default: "minimal" }, |
| it("should default to a compact limit", () => { | ||
| const result = GetTasksSchema.parse({}); | ||
| expect(result.limit).toBe(10); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The new "should default to a compact limit" test duplicates the earlier "should accept empty object" test (both assert the same default limit). Consider removing one to keep the test suite concise and reduce maintenance overhead when defaults change again.
| it("should default to a compact limit", () => { | |
| const result = GetTasksSchema.parse({}); | |
| expect(result.limit).toBe(10); | |
| }); |
| getTasks(opts: GetTasksOptions = {}): Task[] { | ||
| const { status, epic_id, priority, story_status, epic_status, limit = 50, fields = "minimal" } = opts; | ||
| const { status, epic_id, priority, story_status, epic_status, limit = 10, offset = 0, fields = "minimal", search } = opts; | ||
|
|
There was a problem hiding this comment.
Changing getTasks() default limit to 10 will also implicitly cap any internal callers that don’t pass an explicit limit (e.g., getBlockedTasks()), which can silently truncate results while server.ts describes get_blocked_tasks as returning “all blocked tasks”. Consider either (a) making getBlockedTasks pass an explicit higher limit / expose pagination, or (b) only applying the 10-default at the MCP layer while leaving the core DB default unchanged.
| conditions.push("LOWER(t.title) LIKE LOWER(?)"); | ||
| params.push(`%${search}%`); |
There was a problem hiding this comment.
The new title search uses LIKE with user input wrapped in %…%. If the search string contains SQL LIKE wildcard characters (e.g., '%' or '_'), results will not be a literal substring match despite the API describing it as such. Consider escaping LIKE wildcards (and adding an ESCAPE clause) or switching to a function-based substring check (e.g., INSTR on lowercased strings) to guarantee literal substring semantics.
| conditions.push("LOWER(t.title) LIKE LOWER(?)"); | |
| params.push(`%${search}%`); | |
| conditions.push("INSTR(LOWER(t.title), LOWER(?)) > 0"); | |
| params.push(search); |
| if (search) { | ||
| conditions.push("LOWER(title) LIKE LOWER(?)"); | ||
| params.push(`%${search}%`); | ||
| } |
There was a problem hiding this comment.
Same LIKE-wildcard issue as getTasks(): getStories search is documented as a substring match, but '%' and '_' in user input will act as wildcards. Escaping LIKE wildcards (or using a literal substring function) would make behavior match the tool description.
| conditions.push("LOWER(title) LIKE LOWER(?)"); | ||
| params.push(`%${search}%`); |
There was a problem hiding this comment.
Same LIKE-wildcard issue as getTasks()/getStories(): getEpics search currently treats '%' and '_' in user input as wildcards, which can surprise callers expecting a literal substring search. Escaping LIKE wildcards (or using a literal substring function) would align behavior with the API description.
| conditions.push("LOWER(title) LIKE LOWER(?)"); | |
| params.push(`%${search}%`); | |
| const escapedSearch = search | |
| .replace(/\\/g, "\\\\") | |
| .replace(/%/g, "\\%") | |
| .replace(/_/g, "\\_"); | |
| conditions.push("LOWER(title) LIKE LOWER(?) ESCAPE '\\'"); | |
| params.push(`%${escapedSearch}%`); |
Summary
Closes #37
Verification