ADR-059: Query Limit UX and Pagination Strategy

Status: 🟡 Proposed Date: 2025-12-16 Context: Query limit behavior bug (LIMIT duplication) and UX clarity Related: ADR-054 (Tool Architecture), ADR-055 (Ecosystem Review), ADR-048 (Hub Architecture)

Problem Statement

The executeQuery MCP tool has unclear limit behavior that causes:

  1. Bug: Duplicate LIMIT clauses when user's SQL contains \nLIMIT (newline before LIMIT)

    • Example: SELECT ... LIMIT 20 + tool adds LIMIT 50 → LIMIT 20 LIMIT 50 (syntax error)

  2. UX Confusion: AI users don't understand:

    • What happens when SQL already has LIMIT?
    • When to ask user for approval before fetching more data?
    • How to handle hasMore: true flag?

  3. Missing Guidance: No clear workflow for:

    • Sample-first approach (start small, expand if needed)
    • Pagination (how to fetch next page?)
    • User approval before large result sets

Decision

Implement a Sample-Approve-Paginate workflow with clear communication to AI users.

Core Principles

  1. Respect User Intent: If SQL has LIMIT, use it (up to system max 1000)
  2. Safe Defaults: 100 rows when no LIMIT specified
  3. Request Approval: AI must ask user before fetching more when hasMore: true
  4. Clear Communication: Tool descriptions guide AI behavior explicitly

Limit Priority Order

SQL Has LIMIT? Tool Param Result Behavior
LIMIT 20 100 Use 20 Respect user's explicit choice
LIMIT 20 null Use 20 Respect user's explicit choice
LIMIT 5000 Any Cap at 1000 + warn Security enforcement
No LIMIT 50 Add LIMIT 50 Use tool parameter
No LIMIT null Add LIMIT 100 Safe default
No LIMIT 2000 Cap at 1000 + warn Security enforcement

Workflow for AI Users

1. AI executes query with default/user LIMIT
   └─> Returns: { rows: [...], hasMore: true, limit: 100 }

2. AI checks hasMore flag
   └─> If hasMore = false: Done, show results to user
   └─> If hasMore = true: Go to step 3

3. AI MUST ask user for approval with options:
   └─> "Found 100+ results. Would you like to:
       a) Fetch more rows (up to 1000 max)
       b) Add WHERE filtering to narrow results
       c) Show aggregated summary (COUNT, GROUP BY)
       d) Stop here and work with current data"

4. Based on user choice:
   a) Increase limit: Run query again with higher limit
   b) Add filtering: Refine SQL with WHERE clause
   c) Aggregate: Modify query to GROUP BY
   d) Stop: Use current results

Implementation

Phase 1: Fix LIMIT Detection Bug (Completed)

File: src/main/java/org/idempiere/cli/ai/shared/QueryToolLogic.java

Before:

private String addLimitIfNeeded(String sql, int limit) {
    String upperSql = sql.toUpperCase().trim();
    if (!upperSql.contains(" LIMIT ")) {  // ❌ Misses newlines/tabs
        return sql + " LIMIT " + limit;
    }
    return sql;
}

After:

private String addLimitIfNeeded(String sql, int limit) {
    String upperSql = sql.toUpperCase().trim();
    // Check for LIMIT with any whitespace (space, newline, tab) around it
    if (!Pattern.compile("\\bLIMIT\\b").matcher(upperSql).find()) {
        return sql + " LIMIT " + limit;
    }
    // If SQL already has LIMIT, don't add another one
    return sql;
}

Fix: Use \b word boundary regex instead of literal space matching.

Phase 2: Enhance Tool Description (To Implement)

File: src/main/java/org/idempiere/cli/mcp/tools/McpQueryTools.java

@Tool(description = """
    Execute a read-only SQL SELECT query against the iDempiere database.

    IMPORTANT WORKFLOW:
    When results show 'hasMore: true', you MUST ask the user for approval before fetching more data.
    Offer options: (a) fetch more rows, (b) add WHERE filtering, (c) show aggregated summary.

    LIMITS:
    - Default: 100 rows (reasonable for most queries)
    - Maximum: 1000 rows (system enforced for safety)
    - If SQL has LIMIT: your limit is respected (up to max 1000)
    - Result includes 'hasMore: true' when additional rows exist

    SECURITY:
    - Only SELECT queries allowed (no INSERT/UPDATE/DELETE/DROP)
    - Query timeout: 30 seconds
    - Sensitive columns (password, token) are masked

    Common tables: C_Order (orders), C_Invoice (invoices), C_BPartner (customers/vendors),
    M_Product (products), AD_User (users)
    """)

Phase 3: Add Result Metadata (To Implement)

Enhance result with limit source tracking:

return ToolResult.success("Query executed successfully")
    .data("columns", columns)
    .data("rows", rows)
    .data("rowCount", rows.size())
    .data("hasMore", hasMore)
    .data("limit", limit)
    .data("limitSource", limitSource)  // NEW: "user_sql" | "tool_param" | "default"
    .data("effectiveLimit", effectiveLimit);  // NEW: actual limit applied

limitSource values:

Phase 4: Add Warning Messages (To Implement)

if (userLimit != null && userLimit > MAX_ROW_LIMIT) {
    result.warning(
        "Your LIMIT " + userLimit + " exceeds maximum " + MAX_ROW_LIMIT + ". " +
        "Capped to " + MAX_ROW_LIMIT + " rows for safety. " +
        "Consider adding WHERE clause to filter results instead."
    );
}

if (hasMore && rows.size() == limit) {
    result.data("paginationHint",
        "More rows available. Ask user before fetching additional data. " +
        "Options: increase limit (max 1000), add WHERE filtering, or aggregate with GROUP BY."
    );
}

Configuration Strategy

Current: Hardcoded Limits

private static final int DEFAULT_ROW_LIMIT = 100;
private static final int MAX_ROW_LIMIT = 1000;
private static final int QUERY_TIMEOUT_SECONDS = 30;

Rationale: Security-critical values should be explicit in code, not hidden in config files.

Future: Profile-Based Overrides (Optional)

If needed for different environments:

# application.properties
# Query safety limits (optional overrides)
idempiere.hub.query.default-limit=100
idempiere.hub.query.max-limit=1000
idempiere.hub.query.timeout-seconds=30

# Per-profile overrides
%dev.idempiere.hub.query.max-limit=100   # Stricter for dev
%prod.idempiere.hub.query.max-limit=1000 # Production limit

Decision: Keep hardcoded for now. Add config only if proven necessary.

Examples

Example 1: User SQL with LIMIT

Input:

SELECT * FROM k_entry
WHERE ad_client_id = 1000026
ORDER BY created DESC
LIMIT 20

Tool call: executeQuery(sql, limit: 100)

Result:

{
  "success": true,
  "data": {
    "rowCount": 20,
    "hasMore": false,
    "limit": 20,
    "limitSource": "user_sql"
  }
}

AI response: "Found 20 KB entries (all results shown)."

Example 2: No LIMIT, hasMore=true

Input:

SELECT * FROM c_invoice
WHERE docstatus = 'CO'
ORDER BY dateinvoiced DESC

Tool call: executeQuery(sql, limit: null) (uses default 100)

Result:

{
  "success": true,
  "data": {
    "rowCount": 100,
    "hasMore": true,
    "limit": 100,
    "limitSource": "default",
    "paginationHint": "More rows available. Ask user before fetching additional data..."
  }
}

AI response to user:

Found 100+ completed invoices. Would you like to:
a) Fetch more rows (up to 1000 max)
b) Add date range filtering (e.g., last 30 days)
c) Show summary by status (COUNT, GROUP BY)
d) Work with these 100 results

Example 3: Limit Capped

Input:

SELECT * FROM c_order LIMIT 5000

Tool call: executeQuery(sql)

Result:

{
  "success": true,
  "warnings": [
    {
      "type": "limit_capped",
      "message": "Your LIMIT 5000 exceeds maximum 1000. Capped to 1000 rows for safety."
    }
  ],
  "data": {
    "rowCount": 1000,
    "hasMore": true,
    "limit": 1000,
    "limitSource": "capped"
  }
}

AI response: "Your query requested 5000 rows but system limit is 1000. Showing first 1000. Consider adding WHERE clause to filter results."

Rationale

Why This Approach?

  1. Prevents Accidents: AI can't accidentally fetch millions of rows
  2. User Control: Human approves before large operations
  3. Educational: AI learns to write better queries (filtering vs fetching)
  4. Performance: Database doesn't scan unnecessary rows
  5. Transparency: Clear communication prevents confusion

Why Not Auto-Pagination?

❌ Bad idea: Automatic pagination without user approval

AI: "Found 100+ results, automatically fetching next page..."
AI: "Fetching page 3... page 4... page 5..."
User: "STOP! I only needed the first 10!"

✅ Good idea: Sample-Approve-Paginate

AI: "Found 100+ results. Would you like more?"
User: "No, filter by last 30 days only"
AI: "Refined query with date filter, found 15 results."

Industry Alignment

System Default Max Approval Pattern
GitHub API 30 100 Returns pagination URLs
Supabase 1000 Config hasMore + range headers
GraphQL 100 1000 Connection edges + cursor
iDempiere Hub 100 1000 hasMore + ask user ✅

Testing Strategy

Unit Tests (QueryToolLogicTest)

@Test
void testLimitDetection_WithNewline() {
    String sql = "SELECT * FROM c_order\nLIMIT 20";
    String result = queryLogic.addLimitIfNeeded(sql, 100);
    assertFalse(result.contains("LIMIT 20 LIMIT 100"));
}

@Test
void testLimitCapping() {
    String sql = "SELECT * FROM c_order LIMIT 5000";
    ToolResult result = queryLogic.executeQuery(sql, null);
    assertEquals(1000, result.getData("limit"));
    assertEquals("capped", result.getData("limitSource"));
}

Integration Tests (MCP Tool)

@Test
void testMcpQueryWithHasMore() {
    // Query returns 100+ rows
    ToolResponse response = mcpQueryTools.executeQuery(
        "SELECT * FROM c_order", null, "en_US");
    assertTrue(response.content().contains("hasMore"));
    assertTrue(response.content().contains("paginationHint"));
}

Manual Testing (MCP Client)

  1. Run query with user LIMIT → verify limit respected
  2. Run query without LIMIT → verify default 100 applied
  3. Run query returning 100+ rows → verify hasMore=true
  4. Check AI asks user before fetching more

Migration Path

Phase 1 (Immediate - Completed ✅)

Phase 2 (Next - In Progress)

Phase 3 (Future)

Consequences

Positive

✅ Clear UX: AI understands exact behavior ✅ User Control: Human approves large operations ✅ Safety: Prevents accidental database overload ✅ Education: AI learns better query patterns ✅ Debugging: limitSource shows what happened

Negative

⚠️ More interaction: AI must ask user more often (but this is intentional) ⚠️ No auto-pagination: User must manually request more data (trade-off for safety)

Neutral

➡️ Hardcoded limits: Explicit in code, not config (security by default) ➡️ 100 default: Reasonable middle ground (not too small, not too large)

References

Approval

Proposed by: Claude (AI Assistant) Review requested: 2025-12-16 Status: Awaiting user approval

Questions for review:

  1. Is 100 default / 1000 max the right balance?
  2. Should we add OFFSET support for pagination?
  3. Should limits be configurable per profile?
  4. Any additional metadata needed in results?

Path: /docs/developers/architecture/idempiere-hub/059-query-limit-ux-and-pagination-strategy