Skip to content

refactor: add structured output support and MCP spec compliance#960

Merged
matzew merged 3 commits intocontainers:mainfrom
marcnuri-forks:feat/mcp-apps-prereqs
Mar 25, 2026
Merged

refactor: add structured output support and MCP spec compliance#960
matzew merged 3 commits intocontainers:mainfrom
marcnuri-forks:feat/mcp-apps-prereqs

Conversation

@manusa
Copy link
Copy Markdown
Member

@manusa manusa commented Mar 24, 2026

Prerequisite refactors extracted from #920 to reduce its diff size.
These changes are self-contained improvements to the existing codebase
that lay the groundwork for MCP Apps (#753).

Changes

  • PrintObjStructured on Output interface — Extends the Output
    interface with a method that returns both human-readable text and
    optional structured data extracted from Kubernetes objects. Includes
    tableToStructured helper for column-keyed extraction from Table
    responses. Rewrites output tests to testify/suite.

  • NewToolCallResultFull constructor — New API constructor that
    creates a ToolCallResult with separate text and structured content
    fields, for cases where the text representation differs from a JSON
    serialization of the structured data.

  • ensureStructuredObject for MCP spec compliance — The MCP spec
    requires structuredContent to be a JSON object. Slices/arrays
    passed to NewStructuredResult are now automatically wrapped in
    {"items": [...]}.

Refs #753
Refs #920

@manusa manusa force-pushed the feat/mcp-apps-prereqs branch from 8b61a18 to da444aa Compare March 24, 2026 16:24
@manusa manusa marked this pull request as ready for review March 24, 2026 17:29
@manusa manusa added this to the 0.1.0 milestone Mar 24, 2026
Copy link
Copy Markdown
Collaborator

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @manusa !

Left a few thoughts, otherwise this looks like great prep for the MCP apps PR

case *unstructured.UnstructuredList:
items := make([]map[string]any, 0, len(t.Items))
for _, item := range t.Items {
items = append(items, item.Object)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe we deepcopy the item.Object? that way any modifications to PrintResult.Structured won't also modify the Unstructured Object

}
return &PrintResult{Text: text, Structured: items}, nil
case *unstructured.Unstructured:
return &PrintResult{Text: text, Structured: t.Object}, nil
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe we deepcopy the item.Object? that way any modifications to PrintResult.Structured won't also modify the Unstructured Object

// Note: this checks the top-level reflect.Kind, so a pointer-to-slice (*[]T)
// would not be wrapped. All current callers pass value types.
func ensureStructuredObject(v any) any {
rv := reflect.ValueOf(v)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we still are missing handling typed nil, e.g. []string(nil). This would still show up as a rv.Kind() == reflect.Slice, but would end up json serializing to {"items": null} which I don't think is correct

manusa and others added 3 commits March 25, 2026 05:44
Extend the Output interface with a PrintObjStructured method that returns
both a human-readable text representation and optional structured data
extracted from Kubernetes objects. This lays the groundwork for consumers
that need programmatic access to the data alongside the formatted text.

- Add PrintResult struct holding Text and Structured fields
- Implement PrintObjStructured for yaml (returns deep-copied list items
  or object map)
- Implement PrintObjStructured for table (extracts column-keyed maps via
  tableToStructured helper)
- Refactor table.PrintObj to delegate to printTable for code reuse
- Rewrite output tests to testify/suite pattern with new coverage

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Marc Nuri <marc@marcnuri.com>
Add a NewToolCallResultFull constructor that creates a ToolCallResult with
both human-readable text and structured content as separate fields. Unlike
NewToolCallResultStructured (which JSON-serializes structured data into
Content), this preserves the original text representation (e.g. table or
YAML output) alongside the structured data.

Refactor NewToolCallResult and NewToolCallResultStructured to delegate to
NewToolCallResultFull, making it the single canonical constructor.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Marc Nuri <marc@marcnuri.com>
The MCP specification requires structuredContent to marshal to a JSON
object, but NewStructuredResult could receive slice/array values. Add
ensureStructuredObject to automatically wrap slices in {"items": [...]}
so that the result always complies with the spec.

Typed nil slices (e.g. []string(nil)) are detected via reflect and
return nil to avoid producing {"items": null}.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Marc Nuri <marc@marcnuri.com>
@manusa manusa force-pushed the feat/mcp-apps-prereqs branch from da444aa to 5fe1ade Compare March 25, 2026 04:53
Copy link
Copy Markdown
Collaborator

@matzew matzew left a comment

Choose a reason for hiding this comment

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

LGTM

@matzew matzew merged commit dc64bec into containers:main Mar 25, 2026
8 checks passed
@manusa manusa deleted the feat/mcp-apps-prereqs branch March 25, 2026 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants