Skip to content

Feat/aggregates#240

Merged
eskenazit merged 4 commits intomainfrom
feat/aggregates
Apr 10, 2026
Merged

Feat/aggregates#240
eskenazit merged 4 commits intomainfrom
feat/aggregates

Conversation

@jlouvel
Copy link
Copy Markdown
Contributor

@jlouvel jlouvel commented Apr 4, 2026

Related Issue

Closes #191


What does this PR do?

Introduces aggregates[] on the Capability object, enabling DDD-inspired domain functions that can be defined once and referenced from multiple adapters (MCP tools, REST operations) via ref.

Schema:

  • New definitions: Aggregate, AggregateFunction, Semantics
  • ref property on McpTool and ExposedOperation (third anyOf branch alongside simple/orchestrated)
  • name and description are optional when using ref (inherited from the function)

Engine:

  • AggregateRefResolver merges inherited fields from the referenced function (name, description, call, with, steps, inputParameters, outputParameters) — explicit fields override inherited ones
  • Automatic derivation of MCP hints from function semantics (safe → readOnly/destructive, idempotent → idempotent), with explicit hints overriding derived values

Spectral:

  • naftiko-aggregate-function-description rule: warns when a function lacks a description
  • naftiko-aggregate-semantics-consistency rule (custom JS function): detects contradictions between function semantics and MCP tool hints (e.g. safe=true + destructive=true) or REST methods (e.g. safe=true + DELETE)

Documentation:

  • Specification-Schema: §3.4.5 (Aggregate Object), §3.5.5 (McpTool ref mode), §3.9 (ExposedOperation ref mode)
  • FAQ: new "Aggregates & Reuse (DDD-inspired)" section
  • design-guidelines, AGENTS.md, SKILL.md, wrap-api-as-mcp: aggregate design rules and constraints

Built on top of #239 (MCP tool hints).


Tests

  • 25 unit tests (AggregateRefResolverTest): function map indexing, ref lookup, MCP tool merge (name, description, call, inputParams), REST operation merge, deriveHints (6 scenarios), mergeHints (3 scenarios), resolve pipeline
  • 11 integration tests (AggregateIntegrationTest): end-to-end YAML → engine → output for ref resolution, parameter inheritance, hints derivation, overrides, backward compatibility, unknown ref fail-fast
  • 3 Spectral tests (NaftikoSpectralRulesetTest): semantics-hints contradiction detection, REST method contradiction, consistent-document no-warn
  • 3 test fixtures: aggregate-basic.yaml, aggregate-hints-override.yaml, aggregate-invalid-ref.yaml, plus 2 Spectral fixtures

Checklist

  • CI is green (build, tests, schema validation, security scans)
  • Rebased on latest main
  • Small and focused — one concern per PR
  • Commit messages follow Conventional Commits

Agent Context (optional)

agent_name: GitHub Copilot
llm: Claude Opus 4.6
tool: VS Code Chat
confidence: high
source_event: "Issue #191 — Factorize capability core with aggregates[]"
discovery_method: code_review
review_focus: >
  AggregateRefResolver.java (merge logic + hints derivation),
  naftiko-schema.json (McpTool/ExposedOperation anyOf branches),
  aggregate-semantics-consistency.js (Spectral custom function)

@jlouvel jlouvel requested a review from eskenazit April 4, 2026 01:04
@jlouvel jlouvel self-assigned this Apr 4, 2026
@jlouvel jlouvel force-pushed the feat/aggregates branch 3 times, most recently from acd2055 to b221b26 Compare April 7, 2026 14:20
@jlouvel jlouvel force-pushed the feat/aggregates branch 3 times, most recently from bdf0a0a to 0803db9 Compare April 9, 2026 01:45
@jlouvel
Copy link
Copy Markdown
Contributor Author

jlouvel commented Apr 9, 2026

@eskenazit I've rebased from #287 , that might work well

@jlouvel jlouvel requested a review from eskenazit April 9, 2026 14:17
@jlouvel jlouvel force-pushed the feat/aggregates branch 2 times, most recently from ec1bed6 to 4196ced Compare April 9, 2026 18:59
@jlouvel
Copy link
Copy Markdown
Contributor Author

jlouvel commented Apr 9, 2026

@eskenazit I've enhanced this PR to support mocks at the aggregates level. Downgraded JUnit to version 5 due to VS Code detection issue and minor JaCoCo upgrade.

@jlouvel jlouvel force-pushed the feat/aggregates branch 2 times, most recently from 2ae9600 to 55cfb30 Compare April 10, 2026 15:46
jlouvel added 3 commits April 10, 2026 12:11
…ution

Introduce aggregates[] on Capability for defining reusable domain functions

- Schema: Aggregate, AggregateFunction, Semantics definitions; ref on McpTool and ExposedOperation

- Engine: AggregateRefResolver merges ref fields and derives MCP hints from semantics

- Spectral: aggregate-semantics-consistency rule detects contradictions between semantics, MCP hints, and REST methods

- Tests: 25 unit + 11 integration + 3 Spectral tests

- Docs: Specification-Schema, FAQ, design-guidelines, AGENTS.md, SKILL.md, wrap-api-as-mcp

Closes #191

chore: organize test fixtures into subdirectories

feat: add aggregates with DDD-inspired domain functions and ref resolution

Introduce aggregates[] on Capability for defining reusable domain functions

- Schema: Aggregate, AggregateFunction, Semantics definitions; ref on McpTool and ExposedOperation

- Engine: AggregateRefResolver merges ref fields and derives MCP hints from semantics

- Spectral: aggregate-semantics-consistency rule detects contradictions between semantics, MCP hints, and REST methods

- Tests: 25 unit + 11 integration + 3 Spectral tests

- Docs: Specification-Schema, FAQ, design-guidelines, AGENTS.md, SKILL.md, wrap-api-as-mcp

Closes #191

chore: organize test fixtures into subdirectories
Disambiguate ConsumedOutputParameter (name + JsonPath value starting with $)
from aggregate mock functions (name + static/template value) so the
deserializer routes each to the correct field (setMapping vs setValue).

Add shared mock integration test proving MCP and REST adapters produce
identical payloads from a single aggregate function mock definition.
- Add Aggregate, AggregateFunction, FunctionResult engine classes
- Adapters delegate to AggregateFunction at runtime instead of copying fields
- AggregateRefResolver now only validates refs and derives MCP hints
- Move utility classes to engine/util (Resolver, Converter, etc.)
- Move aggregate classes to engine/aggregates
- Move ConsumesImportResolver to engine/consumes
…_modules

fix: use correct schema version in forecast-aggregate example

fix: use correct schema version in test fixtures

feat: add MockOutputParameter to schema for mock aggregate functions
@jlouvel
Copy link
Copy Markdown
Contributor Author

jlouvel commented Apr 10, 2026

@eskenazit As discussed, I have reworked the implementation to have an engine implementation of aggregates

Copy link
Copy Markdown
Contributor

@eskenazit eskenazit left a comment

Choose a reason for hiding this comment

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

Seems way more solid with delegated instances than with copies ! =)

@eskenazit eskenazit merged commit 6d665ba into main Apr 10, 2026
7 checks passed
@eskenazit eskenazit deleted the feat/aggregates branch April 10, 2026 16:33
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.

Factorize capability core with aggregates[]: functions first, entities/events later

2 participants