Consolidate toolspec schema definitions with reusable helpers

[batuhan]

Summary

• Replace verbose inline map[string]any literals with shared property helpers
• Affected schemas: WebSearchSchema, WebFetchSchema, MessageSchema, ImageGenerateSchema, MemorySearchSchema, BeeperDocsSchema
• Add 3 new helpers: StringEnumProperty, BoundedNumberProperty, StringArrayProperty
• Net: -225 lines

Test plan

• go build ./... passes
• go test ./... passes
• go vet ./... passes

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Refactor
  • Improved internal schema construction architecture for better code organization and maintainability across multiple tools.

Walkthrough

Refactored schema definitions in toolspec.go to use reusable helper functions (StringEnumProperty, BoundedNumberProperty, StringArrayProperty) instead of inline map literals. Updated six schema functions to leverage these helpers while preserving existing semantics and required-field logic.

Changes

Cohort / File(s)	Summary
Schema Builder Refactoring pkg/shared/toolspec/toolspec.go	Introduced three new schema property helper functions for consistency. Refactored WebSearchSchema, WebFetchSchema, MessageSchema, ImageGenerateSchema, MemorySearchSchema, and BeeperDocsSchema to use shared constructors instead of inline map literals, reducing duplication while maintaining equivalent output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: consolidating schema definitions by introducing and using reusable helper functions instead of inline literals.
Description check	✅ Passed	The description clearly relates to the changeset by explaining the refactoring approach, listing affected schemas, new helpers added, and test results.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch worktree-agent-aad2ee32

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

pkg/shared/toolspec/toolspec.go (2)

83-87: Consider adding a helper for minimum-only constraints.

maxChars uses an inline map because BoundedNumberProperty requires both bounds. For consistency, consider adding a MinimumNumberProperty helper. Low priority since this is the only occurrence.

♻️ Optional helper addition

func MinimumNumberProperty(description string, minimum float64) map[string]any {
	return map[string]any{
		"type":        "number",
		"description": description,
		"minimum":     minimum,
	}
}

Then replace lines 83-87 with:

-"maxChars": map[string]any{
-	"type":        "number",
-	"description": "Maximum characters to return (truncates when exceeded).",
-	"minimum":     100,
-},
+"maxChars": MinimumNumberProperty("Maximum characters to return (truncates when exceeded).", 100),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/shared/toolspec/toolspec.go` around lines 83 - 87, Add a small helper to
encapsulate number properties with only a minimum bound and replace the inline
map used for "maxChars" with it: implement a MinimumNumberProperty(description
string, minimum float64) that returns a map[string]any with "type":"number",
"description" and "minimum", then update the "maxChars" definition (currently an
inline map in toolspec.go) to call MinimumNumberProperty instead of duplicating
the map; this keeps usage consistent with BoundedNumberProperty and improves
readability.

---

282-282: Minor: description inconsistency for count field.

Other schemas document the range in the description (e.g., "Number of results to return (1-10)"), but this one only mentions the default. Consider aligning for consistency:

-"count":        BoundedNumberProperty("Optional: number of images to generate (default: 1).", 1, 10),
+"count":        BoundedNumberProperty("Optional: number of images to generate (1-10, default: 1).", 1, 10),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/shared/toolspec/toolspec.go` at line 282, Update the description passed
to the BoundedNumberProperty call for the "count" field so it includes the range
like other schemas; change the string in the "count": BoundedNumberProperty(...)
invocation to something like "Number of images to generate (1-10, default: 1)."
so the description documents both range and default consistently.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/shared/toolspec/toolspec.go`:
- Around line 83-87: Add a small helper to encapsulate number properties with
only a minimum bound and replace the inline map used for "maxChars" with it:
implement a MinimumNumberProperty(description string, minimum float64) that
returns a map[string]any with "type":"number", "description" and "minimum", then
update the "maxChars" definition (currently an inline map in toolspec.go) to
call MinimumNumberProperty instead of duplicating the map; this keeps usage
consistent with BoundedNumberProperty and improves readability.
- Line 282: Update the description passed to the BoundedNumberProperty call for
the "count" field so it includes the range like other schemas; change the string
in the "count": BoundedNumberProperty(...) invocation to something like "Number
of images to generate (1-10, default: 1)." so the description documents both
range and default consistently.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 98cda09f-05b7-47e7-a96d-9268e6ca6c02

📥 Commits

Reviewing files that changed from the base of the PR and between ec2b8a9 and eaef315.

📒 Files selected for processing (1)

• pkg/shared/toolspec/toolspec.go

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: Lint
• GitHub Check: build-docker
• GitHub Check: build-agentremote-docker (arm64)
• GitHub Check: build-agentremote-docker (amd64)
• GitHub Check: build-docker
• GitHub Check: Lint

🔇 Additional comments (5)

pkg/shared/toolspec/toolspec.go (5)

69-77: Clean refactoring of WebSearchSchema.

The migration to helper functions improves readability while preserving the schema semantics. The BoundedNumberProperty for count correctly enforces the 1-10 range.

---

134-197: Well-structured refactoring of MessageSchema.

Good decisions made here:

• Simple properties correctly use the new helpers
• Complex nested structures (buttons, card) appropriately remain inline since they have non-trivial schemas
• StringArrayProperty cleanly handles participantIds and targets

---

338-361: Well-designed helper functions.

The three new helpers follow the established patterns:

• StringEnumProperty - correctly constructs enum constraints with []string
• BoundedNumberProperty - uses float64 for JSON number compatibility
• StringArrayProperty - properly defines items schema inline

These provide good abstractions for common JSON Schema patterns without over-engineering.

---

363-373: Good handling of conditional requirement.

The query field is described as "Required unless mode=list" - since JSON Schema's basic required can't express conditional requirements, documenting it in the description is the pragmatic approach. Full enforcement would need if/then/else constructs which would complicate the schema unnecessarily.

---

385-390: LGTM!

Consistent use of BoundedNumberProperty matching the pattern in WebSearchSchema.