Feat/incremental catalog loading#949
Conversation
Introduces CatalogContext to support incremental catalog building in the facade, and updates PromptBuilder to consume it. Includes tests and golden output for the new incremental loading behavior.
The removed test asserted the manifest excludes schema/examples/properties keys, which is already guaranteed by the adjacent test pinning the keys to exactly name+description. Also trims comment noise in the incremental catalog prompt assembly.
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
There was a problem hiding this comment.
Code Review
This pull request introduces an incremental catalog prompt mode to optimize system prompts for LLMs. It adds CatalogContext to manage a compact catalog manifest and load detailed component schemas and examples on demand via a new loadCatalogItems tool. It also updates PromptBuilder to support both full-schema and incremental modes, adjusting the system prompt structure and tool policies accordingly. The reviewer's feedback suggests improving robustness by supporting case-insensitive catalog item lookups, enhancing error handling during example JSON decoding to provide clearer debugging context, and adding corresponding unit tests.
| final Map<String, CatalogItem> byName = { | ||
| for (final item in catalog.items) item.name: item, | ||
| }; | ||
|
|
||
| final seen = <String>{}; | ||
| final details = <CatalogItemDetails>[]; | ||
| for (final name in names) { | ||
| if (!seen.add(name)) continue; | ||
| final CatalogItem? item = byName[name]; | ||
| if (item == null) { | ||
| throw CatalogItemNotFoundException(name, catalogId: catalog.catalogId); | ||
| } |
There was a problem hiding this comment.
To make the catalog item loading more robust against minor casing variations from the LLM (e.g., requesting card instead of Card), consider falling back to a case-insensitive lookup if an exact match is not found. This is a great defensive programming practice for LLM-facing APIs.
| final Map<String, CatalogItem> byName = { | |
| for (final item in catalog.items) item.name: item, | |
| }; | |
| final seen = <String>{}; | |
| final details = <CatalogItemDetails>[]; | |
| for (final name in names) { | |
| if (!seen.add(name)) continue; | |
| final CatalogItem? item = byName[name]; | |
| if (item == null) { | |
| throw CatalogItemNotFoundException(name, catalogId: catalog.catalogId); | |
| } | |
| final Map<String, CatalogItem> byName = { | |
| for (final item in catalog.items) item.name: item, | |
| }; | |
| final Map<String, CatalogItem> byNameLower = { | |
| for (final item in catalog.items) item.name.toLowerCase(): item, | |
| }; | |
| final seen = <String>{}; | |
| final details = <CatalogItemDetails>[]; | |
| for (final name in names) { | |
| if (!seen.add(name)) continue; | |
| final CatalogItem? item = byName[name] ?? byNameLower[name.toLowerCase()]; | |
| if (item == null) { | |
| throw CatalogItemNotFoundException(name, catalogId: catalog.catalogId); | |
| } |
| static List<Object?> _examplesFor(CatalogItem item) { | ||
| return [ | ||
| for (final buildExample in item.exampleData) jsonDecode(buildExample()), | ||
| ]; | ||
| } |
There was a problem hiding this comment.
If a developer-defined example contains invalid JSON, jsonDecode will throw a generic FormatException. Wrapping this exception to include the component's name will make debugging custom catalog items significantly easier.
static List<Object?> _examplesFor(CatalogItem item) {
final examples = <Object?>[];
for (final buildExample in item.exampleData) {
final json = buildExample();
try {
examples.add(jsonDecode(json));
} on FormatException catch (e) {
throw FormatException(
'Failed to parse example JSON for component "${item.name}": ${e.message}',
e.source,
e.offset,
);
}
}
return examples;
}| test('preserves request order and removes duplicates', () { | ||
| final LoadCatalogItemsResult result = CatalogContext.loadItems( | ||
| catalog, | ||
| <String>['Button', 'Card', 'Button'], | ||
| ); | ||
|
|
||
| expect(result.items.map((CatalogItemDetails e) => e.name), <String>[ | ||
| 'Button', | ||
| 'Card', | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
Add a unit test to verify that CatalogContext.loadItems successfully performs case-insensitive lookups and returns the correct casing in the result.
test('preserves request order and removes duplicates', () {
final LoadCatalogItemsResult result = CatalogContext.loadItems(
catalog,
<String>['Button', 'Card', 'Button'],
);
expect(result.items.map((CatalogItemDetails e) => e.name), <String>[
'Button',
'Card',
]);
});
test('supports case-insensitive lookup and returns correct casing', () {
final LoadCatalogItemsResult result = CatalogContext.loadItems(
catalog,
<String>['card', 'TEXT'],
);
expect(result.items.map((CatalogItemDetails e) => e.name), <String>[
'Card',
'Text',
]);
});
Issue: #946
Description
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. For larger changes, raising an issue first helps reduce redundant work.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-devrel channel on Discord.