lelii
/
humainflow-web
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Drive the view-content button from the schema, not from a skills rule 

The button was wired to the literal retriever name "Skills" and to a
hand-built /retriever/Skills/definitions URL, so the node knew about one
particular binding. It now reads x-retriever-definition-url off the item
property: any binding that declares where its value can be read gets the
view, and the node knows nothing about which one it is showing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Lucio Lelii 2026-09-20 20:58:14 +02:00
parent 9802f12203
commit 82396e8b8f
3 changed files with 113 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/app/shared/nodes/generic-node/generic-node.html
View File

@ -392,13 +392,13 @@
<div class="llm-array-item">
<span class="llm-array-item-summary">{{ entry.summary }}</span>
<div class="llm-array-item-actions">
@if (entry.skillId; as skillId) {
@if (entry.definition; as definition) {
<button
type="button"
class="llm-edit-btn"
title="View skill content"
title="View content"
(pointerdown)="$event.stopPropagation()"
(click)="viewSkillContent(skillId, $event)">
(click)="viewItemDefinition(definition, $event)">
<i class="bi bi-eye"></i>
</button>
}
@ -548,13 +548,13 @@
<div class="llm-array-item">
<span class="llm-array-item-summary">{{ entry.summary }}</span>
<div class="llm-array-item-actions">
@if (entry.skillId; as skillId) {
@if (entry.definition; as definition) {
<button
type="button"
class="llm-edit-btn"
title="View skill content"
title="View content"
(pointerdown)="$event.stopPropagation()"
(click)="viewSkillContent(skillId, $event)">
(click)="viewItemDefinition(definition, $event)">
<i class="bi bi-eye"></i>
</button>
}

84
src/app/shared/nodes/generic-node/generic-node.spec.ts
View File

@ -392,33 +392,39 @@ describe('GenericNodeComponent', () => {
});
});
describe('viewing a skill\'s content', () => {
/** The shape `SkillBinding` produces: one property carrying the `@FieldRetriever(name = "Skills")`. */
describe('viewing what a chosen value stands for', () => {
/** The shape `SkillBinding` produces: a value that is an id, plus where to read what it names. */
const skillItemSchema = {
type: 'object',
required: ['skillId'],
properties: {
skillId: { type: 'string', 'x-retriever-name': 'Skills' }
skillId: {
type: 'string',
'x-retriever-name': 'Skills',
'x-retriever-definition-url': '/retriever/Skills/definitions'
}
}
};
it('finds the item property backed by the skills catalog from the schema alone', () => {
it('takes the property and the endpoint from the schema, whatever the binding is called', () => {
const component = fixture.componentInstance as any;
const definitions = component.buildArrayFieldDefinitions({
type: 'object',
properties: {
skills: { type: 'array', items: skillItemSchema }
anything: { type: 'array', items: skillItemSchema }
}
});
expect(definitions).toEqual([expect.objectContaining({ path: 'skills', skillIdProperty: 'skillId' })]);
expect(definitions).toEqual([expect.objectContaining({
path: 'anything',
definitionSource: { property: 'skillId', url: '/retriever/Skills/definitions' }
})]);
});
it('finds it through a $ref, which is the shape the server actually sends', () => {
// Regression: SkillBinding is used by exactly one block type, so the bundler leaves it in
// this schema's own `definitions` and the array references it by $ref rather than inlining
// it - unlike the synthetic schema in the test above.
// Regression: a binding used by one block type stays in that schema's own `definitions`, and
// the array references it by $ref rather than inlining it.
const component = fixture.componentInstance as any;
const definitions = component.buildArrayFieldDefinitions({
@ -429,32 +435,56 @@ describe('GenericNodeComponent', () => {
}
});
expect(definitions).toEqual([expect.objectContaining({ path: 'skills', skillIdProperty: 'skillId' })]);
expect(definitions[0].definitionSource).toEqual({ property: 'skillId', url: '/retriever/Skills/definitions' });
});
it('leaves skillIdProperty null for an array field with no skills-catalog property', () => {
it('offers nothing for a binding whose value declares no definition to read', () => {
// An MCP server's endpoint answers with a JSON schema, not readable text, so it declares none.
const component = fixture.componentInstance as any;
const definitions = component.buildArrayFieldDefinitions({
type: 'object',
properties: {
mcpServers: { type: 'array', items: { type: 'object', properties: { serverName: { type: 'string' } } } }
mcpServers: {
type: 'array',
items: {
type: 'object',
properties: { serverName: { type: 'string', 'x-retriever-name': 'MCPServers' } }
}
}
}
});
expect(definitions[0].skillIdProperty).toBeNull();
expect(definitions[0].definitionSource).toBeNull();
});
it('carries each row\'s skill id into its array item view', () => {
it('carries each row\'s own endpoint and value into its view', () => {
const component = fixture.componentInstance as any;
const definition = {
path: 'skills', label: 'Skills', itemSchema: skillItemSchema, uniqueBy: null, skillIdProperty: 'skillId',
path: 'skills', label: 'Skills', itemSchema: skillItemSchema, uniqueBy: null,
definitionSource: { property: 'skillId', url: '/retriever/Skills/definitions' },
ui: { structural: false, visibleWhen: [], enabledWhen: [] }
};
const items = component.toArrayFieldItems(definition, [{ skillId: 'mcp-context-economy' }]);
expect(items).toEqual([expect.objectContaining({ index: 0, skillId: 'mcp-context-economy' })]);
expect(items).toEqual([expect.objectContaining({
index: 0,
definition: { url: '/retriever/Skills/definitions', value: 'mcp-context-economy' }
})]);
});
it('leaves a row without the view when its value is missing', () => {
const component = fixture.componentInstance as any;
const definition = {
path: 'skills', label: 'Skills', itemSchema: skillItemSchema, uniqueBy: null,
definitionSource: { property: 'skillId', url: '/retriever/Skills/definitions' },
ui: { structural: false, visibleWhen: [], enabledWhen: [] }
};
const items = component.toArrayFieldItems(definition, [{}]);
expect(items[0].definition).toBeNull();
});
it('offers the button in every place the node renders an array row', () => {
@ -462,7 +492,11 @@ describe('GenericNodeComponent', () => {
// fields inside a group fieldset, one for ungrouped fields - and the button was first added
// to only one of them, so it never appeared on an LLM node's ungrouped Skills list.
const component = fixture.componentInstance as any;
const row = { index: 0, summary: 'mcp-context-economy', skillId: 'mcp-context-economy' };
const row = {
index: 0,
summary: 'mcp-context-economy',
definition: { url: '/retriever/Skills/definitions', value: 'mcp-context-economy' }
};
const arrayField = { path: 'skills', label: 'Skills', items: [row] };
// The node recomputes its display model from the (absent) schema on every check, which would
// wipe the rows this test is here to render.
@ -485,17 +519,20 @@ describe('GenericNodeComponent', () => {
}
];
component.cdr.detectChanges();
const buttons = fixture.nativeElement.querySelectorAll('[title="View skill content"]');
const buttons = fixture.nativeElement.querySelectorAll('[title="View content"]');
expect(buttons.length).toBe(2);
});
it('fetches the skill definition and opens it read-only', async () => {
it('reads the endpoint the row carries and opens the answer read-only', async () => {
const component = fixture.componentInstance as any;
const httpMock = TestBed.inject(HttpTestingController);
const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>;
open.mockResolvedValue(null);
const pending = component.viewSkillContent('mcp-context-economy');
const pending = component.viewItemDefinition({
url: '/retriever/Skills/definitions',
value: 'mcp-context-economy'
});
httpMock.expectOne(`${environment.apiUrl}/retriever/Skills/definitions/mcp-context-economy`).flush({
id: 'mcp-context-economy',
name: 'MCP Context Economy',
@ -509,13 +546,16 @@ describe('GenericNodeComponent', () => {
expect(dialog.initial.value).toBe('Prefer write_file over apply_patch when creating a file.');
});
it('shows a message rather than throwing when the skill cannot be loaded', async () => {
it('shows a message rather than throwing when the definition cannot be loaded', async () => {
const component = fixture.componentInstance as any;
const httpMock = TestBed.inject(HttpTestingController);
const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>;
open.mockResolvedValue(null);
const pending = component.viewSkillContent('missing-skill');
const pending = component.viewItemDefinition({
url: '/retriever/Skills/definitions',
value: 'missing-skill'
});
httpMock.expectOne(`${environment.apiUrl}/retriever/Skills/definitions/missing-skill`)
.flush('not found', { status: 404, statusText: 'Not Found' });
await pending;

62
src/app/shared/nodes/generic-node/generic-node.ts
View File

@ -81,13 +81,28 @@ type EditableFieldView = SchemaParameterFieldView<FieldType>;
type OptionalGroupFieldDefinition = SchemaOptionalGroupFieldDefinition;
/**
* An item property whose chosen value is only an id, with the thing it names readable elsewhere.
* The schema says so and says where (`x-retriever-definition-url`), so a row offers to show it
* without the editor knowing what kind of binding it is looking at.
*/
type ItemDefinitionSource = {
property: string;
url: string;
};
/** What one row needs to fetch its own definition: the endpoint, and the value to read there. */
type ArrayItemDefinition = {
url: string;
value: string;
};
type ArrayFieldDefinition = {
path: string;
label: string;
itemSchema: Record<string, any> | null;
uniqueBy: string | null;
/** The item property backed by the skills catalog, if any - the one a "view content" button reads. */
skillIdProperty: string | null;
definitionSource: ItemDefinitionSource | null;
ui: {
structural: boolean;
visibleWhen: UiConditionRule[];
@ -99,7 +114,7 @@ type ArrayFieldDefinition = {
type ArrayFieldItemView = {
index: number;
summary: string;
skillId: string | null;
definition: ArrayItemDefinition | null;
};
type ArrayFieldView = {
@ -928,7 +943,7 @@ export class GenericNodeComponent implements OnDestroy {
uniqueBy: typeof childResolved?.['x-ui-unique-by'] === 'string' && String(childResolved['x-ui-unique-by']).trim().length > 0
? String(childResolved['x-ui-unique-by']).trim()
: null,
skillIdProperty: this.findSkillIdProperty(itemSchema, schema ?? {}),
definitionSource: this.findItemDefinitionSource(itemSchema, schema ?? {}),
ui: {
structural: ui.structural,
visibleWhen: ui.visibleWhen,
@ -941,11 +956,15 @@ export class GenericNodeComponent implements OnDestroy {
});
}
/** The one item property, if any, that a `@FieldRetriever(name = "Skills")` binds - see `SkillBinding`. */
private findSkillIdProperty(itemSchema: Record<string, any> | null, rootSchema: Record<string, any>): string | null {
/** The first item property, if any, that declares where the value it holds can be read in full. */
private findItemDefinitionSource(
itemSchema: Record<string, any> | null,
rootSchema: Record<string, any>
): ItemDefinitionSource | null {
if (!itemSchema) return null;
for (const { key, schema } of orderedSchemaPropertyEntries(itemSchema, rootSchema)) {
if (schema?.['x-retriever-name'] === 'Skills') return key;
const url = toStringOrNull(schema?.['x-retriever-definition-url']);
if (url) return { property: key, url: url.trim() };
}
return null;
}
@ -1546,23 +1565,28 @@ export class GenericNodeComponent implements OnDestroy {
this.maybeCreateBlockOnServer();
}
/** Always available, even read-only: seeing a skill's instructions never needs edit rights. */
async viewSkillContent(skillId: string, event?: Event) {
/** Always available, even read-only: reading what a chosen value stands for needs no edit rights. */
async viewItemDefinition(definition: ArrayItemDefinition, event?: Event) {
event?.preventDefault();
event?.stopPropagation();
try {
const skill = await firstValueFrom(
this.http.get<{ id: string; name?: string; content?: string }>(
`${environment.apiUrl}/retriever/Skills/definitions/${encodeURIComponent(skillId)}`
const loaded = await firstValueFrom(
this.http.get<{ name?: string; content?: string }>(
`${this.toApiUrl(definition.url)}/${encodeURIComponent(definition.value)}`
)
);
await this.openReadonlyTextDialog(skill.name ?? skillId, skill.content ?? '');
await this.openReadonlyTextDialog(loaded.name ?? definition.value, loaded.content ?? '');
} catch {
await this.openReadonlyTextDialog(skillId, 'Could not load this skill\'s content.');
await this.openReadonlyTextDialog(definition.value, 'Could not load this content.');
}
}
private toApiUrl(url: string): string {
if (/^https?:\/\//.test(url)) return url;
return `${environment.apiUrl}${url.startsWith('/') ? url : `/${url}`}`;
}
private async openArrayItemEditor(path: string, index: number | null) {
const definition = this.arrayFieldDefinitions.find((field) => field.path === path);
if (!definition || !this.isPathVisible(path)) return;
@ -1921,12 +1945,16 @@ export class GenericNodeComponent implements OnDestroy {
return value.map((item, index) => ({
index,
summary: this.toArrayItemSummary(definition, item, index),
skillId: definition.skillIdProperty && item && typeof item === 'object' && !Array.isArray(item)
? toStringOrNull((item as Record<string, unknown>)[definition.skillIdProperty])
: null
definition: this.toArrayItemDefinition(definition.definitionSource, item)
}));
}
private toArrayItemDefinition(source: ItemDefinitionSource | null, item: unknown): ArrayItemDefinition | null {
if (!source || !item || typeof item !== 'object' || Array.isArray(item)) return null;
const value = toStringOrNull((item as Record<string, unknown>)[source.property]);
return value ? { url: source.url, value } : null;
}
private toArrayItemSummary(definition: ArrayFieldDefinition, item: unknown, index: number) {
if (!item || typeof item !== 'object' || Array.isArray(item)) {
return `Item ${index + 1}`;