Skip to content

feat(jtk): add --name filter to fields list#195

Merged
rianjs merged 1 commit intomainfrom
feat/194-fields-list-name-filter
Apr 2, 2026
Merged

feat(jtk): add --name filter to fields list#195
rianjs merged 1 commit intomainfrom
feat/194-fields-list-name-filter

Conversation

@rianjs
Copy link
Copy Markdown
Contributor

@rianjs rianjs commented Apr 2, 2026

Summary

  • Adds --name flag to jtk fields list for case-insensitive substring filtering on field names
  • AI agents frequently pass --project (which doesn't exist), causing Cobra errors — --name gives a direct way to find fields like jtk fields list --name "story point"
  • Client-side filtering of the global field list, composes with --custom
  • Updated README command docs and integration test cases

Closes #194

Test plan

  • 5 new unit tests: name filter, case insensitivity, no match, JSON output, composition with --custom
  • Updated 3 existing runList test calls for new parameter
  • make build passes
  • make test passes
  • Integration test cases added to integration-tests.md

@rianjs
feat(jtk): add --name filter to fields list
3bb5c97 
Closes #194

AI agents frequently pass `--project` to `jtk fields list` which doesn't
exist, causing Cobra to error and the agent to give up on finding field
IDs. Adding `--name` for case-insensitive substring filtering gives
agents (and humans) a direct way to find fields without piping through
grep.
@rianjs
Copy link
Copy Markdown
Contributor Author

rianjs commented Apr 2, 2026

TDD Assessment

PR: Add --name filter to jtk fields list

Coverage: Sufficient

The logic is simple (client-side substring filter after fetching) and the test suite covers the meaningful cases well.

What's covered:

  • Basic name filtering (matches included, non-matches excluded)
  • Case-insensitive matching (STORY matches Story Points)
  • No-match path returns "No fields found" message
  • JSON output with filter applied
  • Combined --name + --custom flags
  • Flag registration with correct default value
  • Existing tests updated to pass the new nameFilter param

Minor gaps (not blocking):

  • No test for empty string nameFilter behavior (i.e., "" skips filtering). This is implicit in the existing TestRunList_Table, TestRunList_JSON, and TestRunList_Empty tests since they now pass "", but there's no explicit assertion that an empty filter returns all fields. Low risk given the guard is if nameFilter != "".
  • No test for whitespace-only filter (e.g., " ") — would currently pass through to strings.Contains and match nothing, silently returning "No fields found" rather than treating it as an unset filter. Edge case, not a real-world concern.
  • TestRunList_NameFilter_WithCustom comment says "GetCustomFields returns only custom fields" but the mock server doesn't differentiate on the endpoint — the customOnly flag routes to a different API call (GetCustomFields vs GetAllFields). The test effectively validates that name filtering works after the custom-only path, which is the meaningful thing to test. Comment is slightly misleading but test logic is correct.

Verdict: Approve on coverage. The new logic is straightforward and the added tests are proportionate to the change.

monit-reviewer
monit-reviewer approved these changes Apr 2, 2026
View reviewed changes
Copy link
Copy Markdown

@monit-reviewer monit-reviewer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR Review

Reviewed commit: 3bb5c97

Approved with 1 non-blocking suggestion below. Address at your discretion.

Summary

Reviewer Findings
harness-engineering:architecture-reviewer 1
harness-engineering:architecture-reviewer (1 findings)

💡 Suggestion - tools/jtk/internal/cmd/fields/fields_test.go:202

TestRunList_NameFilter_WithCustom doesn't verify that GetCustomFields (vs GetFields) is actually called when customOnly=true. The mock server returns the same response regardless of which endpoint is hit, so a bug in the routing logic (lines 74-78 of fields.go) wouldn't be caught. Consider checking r.URL.Path in the mock handler to confirm the correct API endpoint is called. The routing logic is trivially correct today, so this is a test quality suggestion rather than a blocking issue.

Completed in 2m 22s | $0.56
Field Value
Reviewers hybrid-synthesis, database:sql-reviewer, security:code-auditor, harness-engineering:knowledge-reviewer, harness-engineering:enforcement-reviewer, harness-engineering:architecture-reviewer, harness-engineering:legibility-reviewer
Reviewed by pr-review-daemon · monit-pr-reviewer
Duration 2m 22s (Reviewers: 22s · Synthesis: 47s)
Cost $0.56
Tokens 143.6k in / 7.9k out
Turns 9

tools/jtk/internal/cmd/fields/fields_test.go
testutil.NotContains(t, stdout.String(), "Summary")
}

func TestRunList_NameFilter_WithCustom(t *testing.T) {
Copy link
Copy Markdown

@monit-reviewer monit-reviewer Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Low (harness-engineering:architecture-reviewer): TestRunList_NameFilter_WithCustom doesn't verify that GetCustomFields (vs GetFields) is actually called when customOnly=true. The mock server returns the same response regardless of which endpoint is hit, so a bug in the routing logic (lines 74-78 of fields.go) wouldn't be caught. Consider checking r.URL.Path in the mock handler to confirm the correct API endpoint is called. The routing logic is trivially correct today, so this is a test quality suggestion rather than a blocking issue.

@rianjs rianjs merged commit e2bb055 into main Apr 2, 2026
7 checks passed
@rianjs rianjs deleted the feat/194-fields-list-name-filter branch April 2, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@monit-reviewer monit-reviewer monit-reviewer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(jtk): add --name filter to fields list

2 participants

@rianjs @monit-reviewer