fix(search): make --search-type config-based case-insensitive (#569) - #580
Conversation
|
|
||
| `kbagent search QUERY` searches across **all item types** (tables, buckets, configs, flows, data apps, transformations) via the Storage API global-search endpoint. It is distinct from `kbagent config search --query Q` which scans only configuration JSON bodies. | ||
| - `search --search-type config-based` delegates to `config search` internally but exposes the unified results shape. | ||
| - `search --search-type config-based` delegates to `config search` internally but exposes the unified results shape. It matches **case-insensitively** (issue #569) while `config search --query` stays case-sensitive unless `-i` is passed -- the same logical table is often mixed-case in a row name and upper-case in `storage.input.tables[].source`, so a case-sensitive body scan answers "is this referenced anywhere?" with a false no. |
There was a problem hiding this comment.
🟡 New behavior note in the AI-agent gotchas file lacks the required version tag
The new case-insensitivity note added to the gotchas reference (gotchas.md:2629) is tagged only with an issue number instead of the mandatory (since vX.Y.Z) version marker, so agents reading it cannot tell which installed versions actually behave that way.
Impact: AI agents may claim case-insensitive config-body search on older installs where the search still misses differently-cased matches.
Repository rule requiring a version tag on every gotcha entry
CONTRIBUTING.md ("Documentation changes (mandatory!)" and "Plugin synchronization map") and CLAUDE.md convention #17 both state that every new/changed behavior entry in plugins/kbagent/skills/kbagent/references/gotchas.md MUST be tagged with (since vX.Y.Z) and that the version tag is non-optional. The added bullet uses (issue #569) only. The current version in pyproject.toml is 0.82.0, so the tag would be (since v0.82.0) (or the release the fix ships in).
Prompt for agents
The new gotchas bullet describing config-based search becoming case-insensitive is tagged with the issue number only. CONTRIBUTING.md and CLAUDE.md convention #17 require every gotcha entry describing new/changed behavior to carry a `(since vX.Y.Z)` tag so agents can version-gate the advice. Add the appropriate since-version tag (the release this fix ships in) to that bullet, keeping the issue reference if desired.
Was this helpful? React with 👍 or 👎 to provide feedback.
| | Mint / rotate / revoke a scoped Storage token (e.g. a device-enrollment token) | `kbagent token create -p P -d DESC [--bucket-write B ...] [--expires-in N]` / `token refresh --token-id ID` / `token delete --token-id ID` (0.66.0+) -- acting token needs `canManageTokens`; secret shown ONCE (persist only `id`+`expires`). Same ops on the SDK facade: `Client.create_scoped_token / refresh_token / delete_token` (+ `create_stream_source`) | -- | assuming a token upload needs `--component-access`/`--can-read-all-file-uploads` (uploads need `--bucket-write` on the sink bucket; those flags gate READING others' uploads, not uploading); telling the user `stream create-source` needs a master token (it uses the normal Storage token) | | ||
| | Search items by name across projects | `kbagent search QUERY [--project P] [--type table\|bucket\|config\|flow\|data-app\|transformation] [--search-type textual\|config-based] [--limit N] [--regex]` (0.30.0+); `--regex` (0.67.0+) opts into case-insensitive whole-term regex on entity names — `report` does NOT match `monthly_report`, write `.*report.*`; textual mode marks `table` results matched via a column name with `matched_columns` in `--json` (0.67.0+; always present, `[]` when the name itself matched; always `[]` under `--regex` — regex never matches column names) | `tool call search_tables` / `tool call search_configurations` (one resource-type per call) | chaining multiple `tool call` for different types; `--regex` with `--search-type config-based` (exit 2); `--regex` below 0.67.0 | | ||
| | Search config JSON bodies | `kbagent search QUERY --search-type config-based [--project P]` (0.30.0+) | `kbagent config search --query Q` (config-body only, no tables/buckets) | repeated `tool call get_config` to grep locally | | ||
| | Search config JSON bodies | `kbagent search QUERY --search-type config-based [--project P]` (0.30.0+; case-insensitive since #569 — a table can be `DCFAmount` in one config and `DCFAMOUNT` in another) | `kbagent config search --query Q` (config-body only, no tables/buckets; case-SENSITIVE unless `-i`) | repeated `tool call get_config` to grep locally | |
There was a problem hiding this comment.
🔍 keboola-expert.md keeps growing while already over its stated 60 KB budget
plugins/kbagent/agents/keboola-expert.md was 61494 bytes at the merge-base and is 61623 bytes after this PR. CONTRIBUTING.md describes a "hard 60 KB prompt budget" for this file (60 KiB = 61440 bytes) and instructs authors to trim stale content rather than grow the file when at the cap. The overrun is pre-existing and small (183 bytes), and there is no CI gate, so this is not a new failure -- but the PR description's "within the 62000 budget" claim uses a looser cap than the documented one. Worth a maintainer decision on whether the documented number is 60000/61440/62000 so the next contributor is not guessing.
Was this helpful? React with 👍 or 👎 to provide feedback.
padak
left a comment
There was a problem hiding this comment.
Review of #580 — fix(search): make --search-type config-based case-insensitive (#569)
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This is a small, well-tested, surgically-scoped fix: SearchService._search_config_based() now passes ignore_case=True to ConfigService.search_configs(), making kbagent search --search-type config-based match case-insensitively, mirroring the already-case-insensitive textual mode. This does resolve issue #569 as reported: the reporter's acceptance criterion was "identical result sets for the same term differing only in casing" — verified below with the reporter's exact scenario (DCFAmount row name vs. DCFAMOUNT physical table id), and confirmed the added regression test fails on pre-fix code and passes after. The design choice to hardcode the behavior (no new --ignore-case flag on kbagent search) rather than add a flag is reasonable and adequately mitigated by the pre-existing kbagent config search --query -i surface, which intentionally stays case-sensitive by default. However, the PR has one concrete, independently-verified BLOCKING problem: its own "within the 62000-byte budget" claim is now stale relative to current main — three other PRs landed on keboola-expert.md since this branch diverged, and a real git merge-tree simulation shows the merged file lands at 62119 bytes, over budget. Overall verdict: REQUEST CHANGES, purely on the stale byte-budget artifact of merge timing, not on the core fix.
Verdict
- Verdict: REQUEST CHANGES
- Blocking findings: 2
- Non-blocking findings: 2
- Nits: 1
Blocking findings
[B-1] plugins/kbagent/agents/keboola-expert.md:100 — merging this PR as-is pushes the file over its 62000-byte hard budget
The PR description claims the file is "now 61623 bytes, within the 62000 budget" — true against the PR's own stale base (merge-base 73ded7a, 61494 bytes), but three PRs have landed on main's copy of this file since this branch diverged (5f5fbae docs(mcp) no-migration-command, 8ee25c9 docs(auth) login-password, and 7445d76 — a dedicated "trim keboola-expert.md back under the 62000-byte budget" commit). Current main's copy is already at 61990 bytes, i.e. 10 bytes of headroom. I ran an actual git merge-tree --write-tree origin/main origin/claude/issue-569-config-search-case-insensitive (a real 3-way merge, not arithmetic) and extracted the resulting blob directly: git show <merged-tree>:plugins/kbagent/agents/keboola-expert.md | wc -c → 62119 bytes. The merge is textually clean (mergeable: MERGEABLE), so nothing will warn the author or a merger — it will silently exceed budget. Per CONTRIBUTING.md ("do not raise the cap... trim stale content first") this is exactly the highest-value silent-drift class keboola-expert.md is called out for. Fix: before merging, rebase onto current main and trim ~120+ bytes elsewhere in the file (or shorten the new "Search config JSON bodies" row text) to stay under budget, then re-verify with wc -c.
[B-2] plugins/kbagent/skills/kbagent/references/gotchas.md:2629 — new case-insensitivity behavior is tagged with an issue number, not a (since vX.Y.Z) version
The amended sentence reads "It matches case-insensitively (issue #569) while config search --query stays case-sensitive..." — no version tag. Per CONTRIBUTING.md ("gotchas.md — always tag with (since vX.Y.Z)... non-optional") and CLAUDE.md convention #17, this is mandatory precisely because an AI agent reading this file cannot tell whether a given user's installed kbagent (pinned via kbagent --json version, per keboola-expert.md Rule 6 VERSION GATE) already has this fix. The enclosing ## ... (since v0.30.0) heading refers to when search became a top-level command, not to this behavior change, so it doesn't cover the gap. This PR contains no version bump (current pyproject.toml is 0.84.0, unreleased) — other recent same-situation PRs in this repo's history (e.g. the login-password docs commit 8ee25c9) speculatively tag with the anticipated next version (e.g. 0.81.0+) rather than omitting the tag. Fix: replace "(issue #569)" with a concrete "(since v0.85.0+)" (or whatever the next release turns out to be) — same gap exists more mildly in the keboola-expert.md §2 matrix row and the CLAUDE.md/context.py comments, which are NON-BLOCKING per the reviewer severity table but worth fixing in the same pass.
Non-blocking findings
[NB-1] src/keboola_agent_cli/services/search_service.py:274 — case-sensitivity on search --search-type config-based is removed with no escape hatch, worth an explicit maintainer sign-off
Verified (focus point 1): ignore_case=True is hardcoded with no CLI flag to override it, so kbagent search --search-type config-based can no longer do a case-SENSITIVE body scan at all. I judge the tradeoff acceptable — the issue explicitly asked for case-insensitive-by-default, the change is a strict superset of prior results (never fewer matches), and kbagent config search --query -i remains the case-sensitive-capable sibling surface for callers who need exact matching. This is not a functional regression, but it is a one-way door: if a future caller genuinely needs case-sensitive matching through the unified search command (e.g. distinguishing Orders from orders configs), there is no way back short of a new flag. Flagging for explicit maintainer agreement rather than blocking, since the PR description already surfaces this tradeoff and explicitly invites disagreement ("happy to flip to docs-only if you disagree").
[NB-2] plugins/kbagent/skills/kbagent/references/gotchas.md:2629 — missing context that --regex was the only prior case-insensitivity workaround, and it's blocked on this surface
Verified (focus point, extra check): commands/search.py:131-134 still rejects --regex combined with --search-type config-based (exit 2, unchanged by this PR) — confirmed via gh pr diff and a direct read of the PR-branch file. Before this fix, users had no workaround at all for case-insensitive config-body search (no --ignore-case flag, and (?i) regex explicitly blocked on this surface) — worth one added clause in the gotchas.md entry so future readers understand why this was a real gap and not just a minor inconvenience. Not blocking; purely additive context.
Nits
[NIT-1]PR description — "No CLI surface change... so no changelog entry" is correct per this repo's actual practice (changelog entries are added at release-bump time, not per-PR, confirmed byCONTRIBUTING.md"Releasing a new version" step 2 and multiple prior release commits) — flagging only so the reviewer/release-owner remembers to backfillchangelog.pyat the next version bump; this PR's behavior change is exactly the kind of thing CONTRIBUTING.md's step 6 wants captured then.
Verification log
gh issue view 569 --repo keboola/cli→ confirmed reporter's exact scenario (mixed-case row nameDCFAmountvs. upper-case Storage tableDCFAMOUNT) and the acceptance bar ("identical result sets... case-insensitive by default, matching the documented textual-search behavior").gh pr view 580 --repo keboola/cli --json title,body,files,additions,deletions,...→ 8 files, +88/-5,state: OPEN, conventionalfix(search):prefix matches a bug fix ✓.gh pr diff 580(202 lines) read in full; confirmed every hunk (CLAUDE.md, keboola-expert.md, commands-reference.md, gotchas.md, context.py, commands/search.py, services/search_service.py, tests/test_search_service.py).- Layer-violation / convention greps (typer-in-services, httpx-in-commands, formatter-in-clients, magic numbers, raw error-code strings, bare except,
print(), token leaks, new baretuple[...]returns) against the full diff → all empty ✓. src/keboola_agent_cli/server/routers/search.py(PR branch) read in full → unchanged by this PR; confirmed it callsregistry.search.search(...)directly (the sameSearchServiceinstance the fix lives in), sokbagent serve's REST/searchendpoint inherits the case-insensitivity fix automatically with zero code change needed — focus point 2 resolved, no gap.src/keboola_agent_cli/commands/search.py:131-134(PR branch) read →--regex+--search-type config-basedstill raisesErrorCode.INVALID_ARGUMENT/ exit 2, unchanged by this PR ✓ (focus point verification).src/keboola_agent_cli/commands/config.py:494-498(PR branch) read →config search --ignore-case/-istill defaults toFalse(case-sensitive by default), confirming the PR description's claim about the sibling surface ✓.- Isolated
git worktree add .../pr580-wt origin/claude/issue-569-config-search-case-insensitive(did NOT touch the shared review worktree or its HEAD) →uv sync --extra server→make check→ exit 0,5449 passed, 12 skipped, 147 deselected(matches the PR description's claimed numbers exactly) ✓. Fullmake checktarget confirmed to run lint + format-check + typecheck + skill-check + version-check + command-sync-check + changelog-check + check-error-codes + check-sentinel-guards + loc-check + tests. pytest tests/test_search_service.py -k config_based -v→ 5 passed ✓.- Reproduced the pre-fix failure directly: temporarily removed the
ignore_case=Trueline fromsearch_service.pyin the isolated worktree and re-ran the two new tests → both FAIL (AssertionError: missed match for 'DCFAmount'), then restored the file (confirmedgit statusclean after) → proves the regression test is real and would have caught the bug, matching the PR description's "verified this test fails on the pre-fix code" claim ✓. - Byte-budget check (focus point 3):
git show origin/main:plugins/kbagent/agents/keboola-expert.md | wc -c→ 61990 (current main, NOT what the PR was authored against).git merge-tree --write-tree origin/main origin/claude/issue-569-config-search-case-insensitive→ clean merge, no conflicts →git show <result>:plugins/kbagent/agents/keboola-expert.md | wc -c→ 62119 bytes, over the 62000 hard budget. See[B-1]. plugins/kbagent/skills/kbagent/references/gotchas.mddiff hunk read → confirmed the new sentence uses "(issue #569)" not a version tag. See[B-2].src/keboola_agent_cli/permissions.py→"search": "read"entry already existed pre-PR (no new command added, so noOPERATION_REGISTRYgap) ✓.gh pr view 580 --json mergeable,mergeStateStatus→mergeable: MERGEABLE,mergeStateStatus: BLOCKED(review-gate, not a merge conflict) — confirms the byte-budget overrun in[B-1]would NOT be surfaced by GitHub's own conflict UI.
Open questions for the author
(none)
… version Two review blockers, both in the plugin doc surfaces. The agent prompt has a hard 62000-byte budget. This branch measured itself against the main it forked from, but main has since grown (#565's auth docs, then a trim back to 61990 B), leaving 10 bytes free -- merging as-is landed at 62119 B. Git cannot see that kind of conflict: both sides edit different lines, the merge is clean, and the invariant breaks anyway. The new note is now the short form, and the matched_columns prose on the row above is condensed the way 7445d76 did it -- same facts, fewer bytes. Merged result: 61985 B. gotchas.md entries carry a `(since vX.Y.Z)` tag, not an issue number: a reader needs to know whether their installed kbagent has the behavior, and #569 does not answer that. This ships in 0.84.0.
What
kbagent search "<term>" --search-type config-basedmatched configuration JSON bodies case-sensitively, while the textual mode of the same command matches case-insensitively.SearchService._search_config_based()calledConfigService.search_configs()withoutignore_case, so the parameter defaulted toFalseand there was no flag onkbagent searchto override it.Why
The same logical table is routinely spelled two ways inside one project: an extractor row named
DCFAmountproduces a Storage tableDCFAMOUNT(typical for SQL Server / Snowflake sources), and a downstream transformation references the upper-case id instorage.input.tables[].source. A case-sensitive scan therefore answers "is this referenced anywhere?" with a false no — the reporter saw 5 vs. 13 org-wide results for the same term differing only in casing, with the mixed-case query missing an actively-scheduled daily dependency. Silent under-reporting is the worst failure mode for a query people run before deleting or refactoring something.How
SearchService._search_config_based()now passesignore_case=True. One line of behaviour; the matcher itself (ConfigService.search_configs) already supported it.kbagent searchnever exposed case control, andkbagent config search --queryremains the case-sensitive-by-default body-scan surface (it has its own--ignore-case/-i). Adding a third knob to express what a sibling command already expresses did not seem worth the CLI surface.Alternative considered and rejected: the issue offers "or at minimum document the case sensitivity in
--help". That leaves every existing user's past "no results" conclusions wrong, so aligning the two modes seemed clearly better — happy to flip to docs-only if you disagree.Docs
Updated every silent-drift surface that describes this command:
CLAUDE.mdcommand list,context.pyAGENT_CONTEXT,commands-reference.md,gotchas.md, and thekeboola-expert.mdtool-selection matrix (now 61623 bytes, within the 62000 budget).Testing
test_config_based_finds_differently_cased_table_reference— the reporter's scenario against the real matcher (mocked client, not a mockedConfigService): a config referencingin.c-finance.DCFAMOUNTis found byDCFAmount,DCFAMOUNTanddcfamountalike. Verified this test fails on the pre-fix code.test_config_based_asks_config_service_for_case_insensitive_match— pins the delegation contract so a future refactor cannot silently drop the flag.make checkgreen: 5449 passed, 12 skipped; lint/format/ty/loc-check/command-sync clean.No CLI surface change (no command, flag or error code added), so no changelog entry — the release-time backfill covers it.
Fixes #569