Knowledge base data lookups: typed conditions and full-replacement updates

[mocha06]

Motivation

Fifth phase of the knowledge base milestone (#414): give the SDK, MCP server, and CLI full coverage for pipe-scoped data lookups — the third knowledge base source kind, after plain text (#427) and documents (#428). A data lookup lets an AI agent search cards in a source pipe by conditions and return selected field values, attached to an agent or behavior via dataSourceIds.

What ships

Full SDK → service → client → MCP → CLI → docs → skills → tests, in one PR (one phase = one PR).

MCP tool	CLI command
get_ai_knowledge_base_data_lookup	pipefy kb data-lookup get
create_ai_knowledge_base_data_lookup	pipefy kb data-lookup create
update_ai_knowledge_base_data_lookup	pipefy kb data-lookup update
delete_ai_knowledge_base_data_lookup	pipefy kb data-lookup delete

Registry moves 172 → 176 tools; parity.md, README counts, the knowledge-bases doc page, the AI-agents skill, and the CLI help golden are updated in lockstep.

Proven contract (traced to the owning layer, not just the schema)

Every argument was walked through the full delegation chain (Core GraphQL → the AI chat service's JSON:API → the DataSource model → the agent runtime), not inferred from the schema.

• Auth: reads need read_ai_agents; all writes need manage_ai_agents (same milestone invariant as phases 3/4).
• pipe_uuid: the pipe UUID. data_lookup_id: the data-source UUID (from the list).
• source_repo_id: the numeric pipe ID. The schema types it ID and create-time authorization would accept a UUID, but the agent runtime integer-coerces the stored value — a UUID persists successfully and then breaks the lookup at run time. Enforced client-side as decimal-digits-only.
• name: required. description: required, 1-900 chars — schema-optional but the backing DataSource model rejects a blank one (the shared knowledge base rule).
• output_fields: required, 1-30 entries (backend model cap), each a non-blank field ID (field slugs plus static fields like id, title, created_at).
• conditions: required, non-empty. Each needs field + operator (an opaque backend string, e.g. "eq", "contains") and is one of two shapes:
  • Static: value required, must be a non-blank string. The write path validates only field/operator; a missing or non-string value is accepted by the API and then breaks the lookup at agent run time, so it is enforced client-side.
  • AI-filled: usingFillWithAi: true with inputName, inputType, inputDescription (backend-validated); value must be omitted (the runtime supplies it, so a static value would be silently ignored).
  • Optional passthrough keys attribute/fieldUuid (schema-supported, backend-specific search modes) are accepted so externally-created lookups round-trip; unknown keys are rejected.

Behavior boundaries

• Reads never return conditions — the API stores them but Core's type does not expose them, so callers keep their definition as the client-side source of truth.
• Updates replace the whole definition — the backend rewrites the stored metadata wholesale, so source_repo_id, output_fields, and conditions are required on every update (the complete set, not a delta) and an omitted search_query is cleared. Only name/description are partial.
• search_query is optional everywhere; it is a backend-defined search-mode marker, not free text — callers leave it unset unless they know the backend value they need.
• Deletes require confirmation (MCP confirm=true; CLI --yes or prompt). CLI writes gate on the read-access probe first.

Divergences from the issue text

1. source_repo_id is validated as numeric, not treated as "opaque, no format coercion" — a UUID passes create-time authorization but breaks the lookup at agent run time (see contract above).
2. search_query is optional on update, not required as the issue listed — it is optional at every layer, and requiring it would make lookups without one un-updatable; omission clears it (documented at all three surfaces).

Docs & skills touched

docs/parity.md, docs/mcp/README.md, docs/mcp/tools/knowledge-bases.md (new data-lookup section), README.md (tool count), skills/ai-agents/pipefy-ai-agents/SKILL.md (create-with-AI-filled-condition → attach → full-replacement-update flow).

Testing

Offline gates: full suite (3669 passed), ruff check + format --check, lint-imports, bump_version.py verify, skill-ref + frontmatter linters, parity/remote-seed drift guards, CLI help golden, packaging wheel build. 132 knowledge base tests across SDK/MCP/CLI, including condition typing rules, full-definition update enforcement, confirm-delete, and the client-side limit/format checks (proven in-process rather than through pathological live inputs).

Live end-to-end against pipe 303088927 (dev):

#	Scenario	Expected	Observed	Verdict
1	Access probe	ok, item count	ok: true, count 4, read-only note	✅
2	Create (AI-filled + static conditions, 3 output fields)	created; payload without conditions	id returned, no conditions echoed	✅
3	List includes new item	present, type: data_lookups	present	✅
4	Get by id	full definition minus conditions	matches; no conditions field	✅
5	Full-replacement update (rename, narrow output fields, omit description)	renamed, fields replaced, description kept	as expected	✅
6	Delete without confirm	preview only, nothing deleted	requires_confirmation: true	✅
7	Delete throwaway with confirm=true	success	deleted_id returned	✅
8	Get deleted id	classified not-found + discovery hint	kind: not_found, hint, correlation id	✅
9	Create with inaccessible numeric source pipe	classified rejection	kind: not_found, correlation id	✅
10	Client-side limit/typing checks	ValueError before any network call	proven by unit tests	✅

[adriannoes]

Summary

Solid fifth KB phase: full SDK/MCP/CLI parity for data lookups, typed conditions, full-replacement updates, and confirm-delete, with green CI and a green local unit suite (checkout+tests). Not a merge blocker on behavior; one docs count mismatch would be good to fix before merge (Knowledge bases domain row still at 10 while this PR lands four data-lookup tools and the headline says 176).

What worked well

• Full surface lockstep (registry 176, parity, skills create-attach-full-replacement flow, CLI help golden).
• Client-side contract matching real backend under-validation (numeric source_repo_id, static vs AI-filled conditions, output_fields 1-30), documented at the point of use.
• Deliberate, explained divergences from #414 rather than silent mismatch.

Also noted

• Prefer an ASCII digit check for source_repo_id (same spirit as PIPEFY_ORG_ID / ^[0-9]+$) over str.isdecimal(), which accepts Unicode Nd digits. Not a proven production break; consistency and test coverage if you touch that path.
• Document the list type discriminator data_lookups on get_ai_knowledge_bases (tool docstring, knowledge-bases.md, skill). It is asymmetric vs knowledge_base_plain_texts / knowledge_base_documents.
• AI-filled missing-field errors use snake_case (input_name, …) while public docs teach camelCase (inputName, …). Worth aligning on the next edit of that path.

Review path

• Checked out the PR tip, ran package unit tests and ruff; CI green on tip.
• Subagent passes covered security, architecture, tests, agent UX, and bug hunt; README domain count (10 vs 14 / table sum 172 vs 176) survived refutation as the only inline item.
• Bugbot UUID read-then-update trap was adjusted: create shares the numeric guard; intentional hardening, not a ship blocker.

[mocha06]

Thanks for the thorough pass, @adriannoes — all four points addressed in 0f3c005 (CI green):

• README domain count (blocker): Knowledge bases row is now 14 with a data-lookup blurb; the domain table sums to 176, matching the headline.
• ASCII digit check for source_repo_id: switched from str.isdecimal() to isascii() and isdigit(), so only ASCII 0-9 passes (same spirit as the ^[0-9]+$ convention). The 123² unit test covers the Unicode-digit rejection.
• data_lookups list discriminator: documented on get_ai_knowledge_bases in the tool docstring, knowledge-bases.md, and the skill, calling out that it is unprefixed unlike knowledge_base_plain_texts/knowledge_base_documents.
• AI-filled error casing: the missing-field and misplaced-field errors now name inputName/inputType/inputDescription in camelCase, matching what the tool and docs teach.

On the disclosure note from the testing matrix (upstream Core returns a raw internal error hash that the classifier passes through verbatim on rejection paths): it is pre-existing and shared with the plain-text/document phases, and it is already tracked in the internal bug tracking (1415676177), so I've left it out of scope here.

[adriannoes]

Summary

Approving on tip 0f3c005. The follow-up commit addresses the review notes: Knowledge bases domain count is 14 (table sums to 176), source_repo_id uses an ASCII digit guard, AI-filled validation errors use camelCase aliases, and data_lookups is documented on the list surface. CI is green on tip.

What worked well

• Fast, complete response to the review comments in one focused commit.
• The original data-lookup surface (parity, typed conditions, full-replacement updates, confirm-delete) remains solid.

Thanks for the clean follow-up, @mocha06.

Review path

• Re-checked tip after the review-address commit; verified README domain arithmetic, ASCII source_repo_id guard, camelCase error wording, and data_lookups docs.
• CI lint/test/skill frontmatter green on tip.