-
Notifications
You must be signed in to change notification settings - Fork 71
feat(fiori-mcp-server): Skill productization #5175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
IvoSG
wants to merge
4
commits into
feat/adp-mcp-rta-workflow
Choose a base branch
from
feat/skill-and-knowledge-base
base: feat/adp-mcp-rta-workflow
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@sap-ux/fiori-mcp-server': patch | ||
| --- | ||
|
|
||
| FEAT: Split sap-fiori-adp-controller-extension skill into workflow core and reference files (hitl-gating, actions-reference, example-session) add enhancement to knowledge base | ||
207 changes: 38 additions & 169 deletions
207
...ls/adp-controller-extension-flow/SKILL.md → ...p-fiori-adp-controller-extension/SKILL.md
Large diffs are not rendered by default.
Oops, something went wrong.
29 changes: 29 additions & 0 deletions
29
...-server/skills/sap-fiori-adp-controller-extension/references/example-session.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| # Example Session | ||
|
|
||
| > Referenced from [SKILL.md](../SKILL.md). Worked walkthrough of the full skill flow for a button + dialog on an Object Page. | ||
|
|
||
| **User request:** "Add a custom button to the object page toolbar that shows a dialog with order details." | ||
|
|
||
| This intent maps to **two actions** (see *Disambiguation by intent* in [rta-actions.md](rta-actions.md)): `CTX_ADDXML` for the button, then `CTX_EXTEND_CONTROLLER` for the press handler. The app starts on a List Report, so the Object Page toolbar isn't on screen yet — the page-action loop drives the navigation first. | ||
|
|
||
| 1. `start` with `{ site, frameId: "preview" }` → `{ site, frameId, rtaStarted: true }` | ||
| 2. `get_page_actions` → `registered: [{ id: "loadData", … }]`. The user's target is on the Object Page, so the table needs rows first. | ||
| 3. `call_page_action({ id: "loadData" })` → `result: { status: "ok" }`. Filter Bar search ran and rows arrived. | ||
| 4. `get_page_actions` → `registered` now includes `navigateToRow`. | ||
| 5. `call_page_action({ id: "navigateToRow" })` → `result: { status: "ok" }`. The Object Page is mounted. | ||
| 6. `get_overlays` → find the toolbar control (confidence 0.88 → High → silent). The overlay's `actionIds` includes `CTX_ADDXML` and `CTX_EXTEND_CONTROLLER`; `actionsCatalog` has the parameter schemas. | ||
| 7. **Iteration 1 — add the button:** | ||
| - `get_context` for `(<toolbar>, CTX_ADDXML)` → returns `parentAggregationName`, `aggregationsByClass` (with `content.contentLength`), `availableModels` | ||
| - Action confidence 0.92 (High; alternatives considered: only fragment-add was in `actionIds`) | ||
| - Payload `{ fragmentPath: "fragments/OrderDetailsButton.fragment.xml", targetAggregation: "content", index: <contentLength> }` confidence 0.91 → High | ||
| - `call_action` → `success: true` | ||
| 8. **Iteration 2 — add the controller extension:** | ||
| - The same overlay's `actionIds` still includes `CTX_EXTEND_CONTROLLER` | ||
| - `get_context` for `(<toolbar>, CTX_EXTEND_CONTROLLER)` → returns `viewId` | ||
| - Payload `{ codeRef: "coding/OrderDetailsExt.js", viewId: "<viewId>" }` confidence 0.93 → High | ||
| - `call_action` → `success: true` | ||
| 9. `save` → `saved: true` | ||
| 10. `adp_controller_extension` Phase 1 → knowledge base | ||
| 11. Generate fragment XML (`OrderDetailsButton.fragment.xml`) + controller extension (`OrderDetailsExt.js` with the press handler that opens the dialog), Phase 2 writes files | ||
| 12. `restart` → `{ site, frameId, rtaStarted: true }`. Navigate to the Object Page toolbar again via `get_page_actions` / `call_page_action`, then `get_overlays` — confirm the inserted fragment overlay (`OrderDetailsButton`) appears. | ||
| 13. `stop`, then kill the editor server. Report done — including the confidence the model assigned to each AI decision. |
83 changes: 83 additions & 0 deletions
83
...-mcp-server/skills/sap-fiori-adp-controller-extension/references/hitl-gating.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| # Confidence & HITL Gating | ||
|
|
||
| > Referenced from [SKILL.md](../SKILL.md). The workflow steps refer back here for thresholds and rules. | ||
|
|
||
| Three steps in this workflow are AI judgment calls, not deterministic lookups: control selection (Step 4), action selection (Step 5), and payload preparation (Step 8). Wrong choices at these points either edit the wrong UI or silently corrupt the change. To make HITL reliable, **rate every such decision with a self-assessed confidence in `[0, 1]`** and gate behavior on per-decision thresholds. | ||
|
|
||
| ## Confidence rubric | ||
|
|
||
| Anchor your self-rating to evidence, not vibes: | ||
|
|
||
| | Confidence | When to assign it | | ||
| |---|---| | ||
| | **0.95–1.00** | Exact, unambiguous match. Single candidate. Wording in the user's instruction maps 1:1 to one option. | | ||
| | **0.85–0.94** | Strong match. Top candidate is clearly best; runner-up is materially worse. All required fields derived from explicit context or instructions. | | ||
| | **0.65–0.84** | Likely match. Top candidate is plausible but the runner-up is also reasonable, or one non-critical field had to be inferred. | | ||
| | **0.40–0.64** | Weak match. Several plausible candidates, or a required field was inferred from weak signals. | | ||
| | **< 0.40** | No real match. Don't pick — list options and ask. | | ||
|
|
||
| ## Three bands → three behaviors | ||
|
|
||
| | Band | Range | Behavior | | ||
| |---|---|---| | ||
| | **High** | ≥ high threshold | Proceed silently. Record the choice + confidence in the final summary. | | ||
| | **Medium** | ask threshold ≤ x < high threshold | Proceed but **announce** the choice on one line: `Using <choice> (confidence 0.78). Continuing — interrupt to change.` Do not stop. | | ||
| | **Low** | < ask threshold | **Stop and ask.** Present the top 2–3 ranked candidates with their confidences. Never guess. | | ||
|
|
||
| ## Medium-band lock-in (critical) | ||
|
|
||
| A Medium-band announcement is a **soft commitment, not a draft**. The next tool call must use the announced choice exactly. If downstream evidence later invalidates the choice — for example, the overlay's `actionIds` doesn't contain the action you expected, `get_context` returns a structure that doesn't match, or the action call errors — you MUST: | ||
|
|
||
| 1. **Stop.** Do not silently switch to a different control, action, or payload. The user already saw "Using X" and is reasonably expecting X. | ||
| 2. **Report the contradiction explicitly:** what you announced, what came back, and what that means. | ||
| 3. **Ask the user how to proceed.** Offer concrete alternatives where possible (e.g. "(a) try a different control, (b) use a different action on the same control, (c) stop"). | ||
|
|
||
| Silently revising a Medium-band choice is the single worst HITL failure mode this skill protects against. The user's "interrupt to change" affordance is real-time only; once you've moved past it, ask explicitly before changing course. | ||
|
|
||
| ## Expected-action absence is a hard stop | ||
|
|
||
| When you derived an expected action id in Step 5 from the user's intent (e.g. "add a button" → `CTX_ADDXML`) and the chosen control doesn't expose that action, **this is an ask point, not a search heuristic**. Do not silently iterate to a different control hoping the action appears. Instead: | ||
|
|
||
| 1. **Stop.** Tell the user, in plain terms, that the control they're working with doesn't support the action needed for this intent. Name the control and the missing action explicitly. | ||
| 2. **Propose a similar control if one exists.** Look at the overlay list for candidates of the same or compatible `controlType` (e.g. another `OverflowToolbar`, another `Toolbar`, the parent container) and name **one specific alternative** with the reason it's similar. Do not list five — pick the closest one. | ||
| 3. **Wait for explicit confirmation.** Do not switch controls until the user replies. If they confirm, restart from Step 5 (action selection) on the new control and verify the expected action is present *before* announcing. | ||
| 4. **If no similar control is obvious**, ask the user to point at one or to clarify what they meant — don't guess. | ||
|
|
||
| This rule has special weight when the **user explicitly named the control** (e.g. "add a button to the TableToolbar"). In that case the model must not silently substitute a different control under any circumstance — the user said which one, and a missing action means the request itself is impossible as stated, which the user must be told. | ||
|
|
||
| This rule applies even when the chosen control was selected in the High band. A High-band control plus an unexpected action set is a higher-priority signal than the original control-selection confidence. | ||
|
|
||
| Phrasing template: | ||
|
|
||
| > The `<chosenControl>` (`<controlType>`) doesn't expose `<expectedAction>` — only `<actual ids>`. | ||
| > | ||
| > The closest similar control on this page is `<proposedControl>` (`<controlType>`), because `<reason>`. Should I switch to `<proposedControl>` and continue, or did you mean a different control? | ||
|
|
||
| ## Per-decision thresholds | ||
|
|
||
| | Decision | High ≥ | Ask < | Reasoning | | ||
| |---|---|---|---| | ||
| | Step 4 — Control selection | 0.95 | 0.60 | Cheap to undo if wrong (the action will fail or look obviously wrong). | | ||
| | Step 5 — Action selection | 0.85 | 0.65 | Few options, usually obvious; bump slightly higher because the wrong action causes a wrong *kind* of change. | | ||
| | Step 8 — Payload preparation | 0.90 | 0.70 | **Highest risk.** Action can succeed yet produce a broken/misplaced change. Bias toward asking. | | ||
|
|
||
| ## Ambiguity overrides confidence (must ask) | ||
|
|
||
| The Medium band is for "I know which one and the runner-up is materially worse, but not by a wide margin." It is **not** for "two candidates look interchangeable." When two or more candidates are roughly equally plausible — same `controlType`, similar labels, both reasonable matches for the user's words — that is a **disambiguation problem, not a confidence problem**. Treat it as a hard ask regardless of the score: | ||
|
|
||
| - Two or more candidates within **0.10** confidence of each other → **stop and ask**, even if the top score is in the High band on paper. | ||
| - "I'll pick the more conventional one" or "this is where row actions live" reasoning is not a tiebreaker — it's a guess. List the candidates and ask. | ||
|
|
||
| Concrete example: a ListReport page has both a `TableToolbar` and a `FooterToolbar`. The user said "the toolbar." Both are `sap.m.OverflowToolbar`. Don't pick — ask. | ||
|
|
||
| ## Required-field rule (Step 8) | ||
|
|
||
| A required field whose value cannot be derived from (a) the action's payload schema, (b) the element context, or (c) explicit user instructions **caps the whole-payload confidence at 0.55** regardless of how strong the other fields are. That puts payload prep into the "ask" band by default whenever guessing is required. | ||
|
|
||
| ## Multi-change runs | ||
|
|
||
| When executing many changes in one session, the cumulative chance of a wrong silent decision grows. **For runs with more than 3 changes, be more conservative: prefer announcing choices rather than proceeding silently, and always re-run `get_overlays` between iterations — confidence drops if the snapshot is stale.** | ||
|
IvoSG marked this conversation as resolved.
|
||
|
|
||
| ## Reporting | ||
|
|
||
| The final summary (Step 14) must include, per change: chosen control, action, and the confidence the model assigned to each AI decision. This makes silent high-confidence decisions auditable after the fact. | ||
31 changes: 31 additions & 0 deletions
31
...-mcp-server/skills/sap-fiori-adp-controller-extension/references/rta-actions.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # RTA Actions | ||
|
|
||
| > Referenced from [SKILL.md](../SKILL.md). Use the `id` values from this table — **never invent or guess an action id.** If `actionsCatalog` exposes ids not listed here, surface them to the user rather than picking. | ||
|
|
||
| ## `CTX_ADDXML` — Add: Fragment | ||
|
|
||
| Insert an XML fragment as a child of the selected control. This is the action for "add a button / field / column / section" via a fragment file. | ||
|
|
||
| | Field | Type | Required | Description | | ||
| |---|---|---|---| | ||
| | `fragmentPath` | string | yes | Path to the fragment XML, formatted `fragments/<Name>.fragment.xml`. | | ||
| | `targetAggregation` | string | yes | Aggregation of the parent control where the fragment is inserted (e.g. `content`, `items`, `headerContent`). Read from `get_context`. | | ||
| | `index` | int | yes | Position within the aggregation. Use `0` for first, the current child count for "append at end". | | ||
|
|
||
| ## `CTX_EXTEND_CONTROLLER` — Extend Controller | ||
|
|
||
| Attach a controller extension JS file to a view. Use this when fragment event handlers reference methods that need a JS implementation, or whenever the user asks for behavior changes (handlers, lifecycle hooks, formatters). | ||
|
|
||
| | Field | Type | Required | Description | | ||
| |---|---|---|---| | ||
| | `codeRef` | string | yes | Path to the controller extension, formatted `coding/<Name>.js`. | | ||
| | `viewId` | string | yes | Any control id inside the view, or the view id itself. The `controlId` of the current selection works. | | ||
| | `instanceSpecific` | boolean | no | `true` extends only this view instance; `false`/omitted extends every view that uses this controller. | | ||
|
|
||
| ## Disambiguation by intent | ||
|
|
||
| | User intent | Action | | ||
| |---|---| | ||
| | "add a button / field / column / section / dialog opener" | `CTX_ADDXML` (fragment carries the new control) | | ||
| | "make this button do X" / "open a dialog when …" / "change behavior" | `CTX_EXTEND_CONTROLLER` (handler lives in the controller extension) | | ||
| | "add a button that opens a dialog" | **Both**, in this order: `CTX_ADDXML` for the button, then `CTX_EXTEND_CONTROLLER` for the press handler. Run as two separate iterations of Steps 4–9. | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.