From 13b3b324f7ba97238231d8a15490bb878adf5087 Mon Sep 17 00:00:00 2001 From: Jaime Wren Date: Wed, 4 Mar 2026 08:36:37 -0800 Subject: [PATCH] refactoring: Implement resource fetching service, add new skill definitions, and introduce prompts file. --- .../src/commands/generate_skill_command.dart | 9 +- .../src/commands/validate_skill_command.dart | 9 +- tool/lib/src/services/gemini_service.dart | 118 +---------- tool/lib/src/services/prompts.dart | 62 ++++++ .../services/resource_fetcher_service.dart | 84 ++++++++ .../fetch_and_convert_content_test.dart | 65 ------ tool/test/services/gemini_service_test.dart | 162 --------------- .../resource_fetcher_service_test.dart | 188 ++++++++++++++++++ 8 files changed, 355 insertions(+), 342 deletions(-) create mode 100644 tool/lib/src/services/prompts.dart create mode 100644 tool/lib/src/services/resource_fetcher_service.dart delete mode 100644 tool/test/commands/fetch_and_convert_content_test.dart create mode 100644 tool/test/services/resource_fetcher_service_test.dart diff --git a/tool/lib/src/commands/generate_skill_command.dart b/tool/lib/src/commands/generate_skill_command.dart index 5b3bac5..1b2b573 100644 --- a/tool/lib/src/commands/generate_skill_command.dart +++ b/tool/lib/src/commands/generate_skill_command.dart @@ -9,6 +9,7 @@ import 'package:path/path.dart' as p; import '../models/skill_params.dart'; import '../services/gemini_service.dart'; +import '../services/resource_fetcher_service.dart'; import 'base_skill_command.dart'; /// Command to generate skills from a configuration file. @@ -46,10 +47,12 @@ class GenerateSkillCommand extends BaseSkillCommand { } try { - final combinedMarkdown = await fetchAndConvertContent( + final fetcher = ResourceFetcherService( + httpClient: httpClient, + logger: logger, + ); + final combinedMarkdown = await fetcher.fetchAndConvertContent( skill.resources, - httpClient, - logger, configDir: configDir, ); diff --git a/tool/lib/src/commands/validate_skill_command.dart b/tool/lib/src/commands/validate_skill_command.dart index 9e2e191..893dbd3 100644 --- a/tool/lib/src/commands/validate_skill_command.dart +++ b/tool/lib/src/commands/validate_skill_command.dart @@ -9,6 +9,7 @@ import 'package:path/path.dart' as p; import '../models/skill_params.dart'; import '../services/gemini_service.dart'; +import '../services/resource_fetcher_service.dart'; import 'base_skill_command.dart'; /// Command to validate skills by re-generating and comparing with existing skills. @@ -52,10 +53,12 @@ class ValidateSkillCommand extends BaseSkillCommand { try { // Re-generate markdown content - final markdown = await fetchAndConvertContent( + final fetcher = ResourceFetcherService( + httpClient: httpClient, + logger: logger, + ); + final markdown = await fetcher.fetchAndConvertContent( skill.resources, - httpClient, - logger, configDir: configDir, ); diff --git a/tool/lib/src/services/gemini_service.dart b/tool/lib/src/services/gemini_service.dart index c7144f0..e79c063 100644 --- a/tool/lib/src/services/gemini_service.dart +++ b/tool/lib/src/services/gemini_service.dart @@ -8,11 +8,10 @@ import 'package:google_cloud_ai_generativelanguage_v1beta/generativelanguage.dar import 'package:http/http.dart' as http; import 'package:logging/logging.dart'; import 'package:meta/meta.dart'; -import 'package:path/path.dart' as p; import 'package:retry/retry.dart'; import 'package:yaml_writer/yaml_writer.dart'; -import 'markdown_converter.dart'; +import 'prompts.dart'; import 'skill_instructions.dart'; /// Service for interacting with the Gemini API to generate and validate skills. @@ -71,7 +70,7 @@ class GeminiService { }) async { final service = GenerativeService(client: _client); final lastModified = io.HttpDate.format(DateTime.now()); - final prompt = _createSkillPrompt(rawMarkdown, instructions); + final prompt = Prompts.createSkillPrompt(rawMarkdown, instructions); final request = _createRequest( prompt, @@ -128,32 +127,13 @@ class GeminiService { int thinkingBudget = defaultThinkingBudget, }) async { final service = GenerativeService(client: _client); - final validationPrompt = - ''' -Validate the following skill document against the provided source material and verify if it is valid. -Focus on: -1. Accuracy: Does the skill capture the technical details correctly based on the Source Material? -2. Structure: Is the skill well-structured according to skill best practices? -3. Completeness: Is any critical information missing in the skill that is present in the Source Material? - -Context: -- The skill was originally generated on: $generationDate -- The current evaluation is using model: $modelName -- The instructions used to generate the skill were: -$instructions - -Source Material: -$markdown - -Current Skill Content: - "$currentSkillContent" ---- - -Grade the current output based on the instructions and the comparison to current website content and instructions today. -Establish a conclusion on whether the new skill is valid or not. -Reasons for a good or bad quality grade should be provided including concepts such as missing content, different model used, more than a few months old, etc. -On the very last line, output "Grade: [0-100]" representing overall quality of the skill compared to the assumed value if it were generated again today. -'''; + final validationPrompt = Prompts.validateExistingSkillContentPrompt( + markdown, + instructions, + generationDate, + modelName, + currentSkillContent, + ); final request = _createRequest( validationPrompt, @@ -223,24 +203,6 @@ On the very last line, output "Grade: [0-100]" representing overall quality of t return '${cleaned.trim()}\n'; } - String _createSkillPrompt(String markdown, String? instructions) { - return ''' -Rewrite the following technical documentation into a high-quality "SKILL.md" file. - -DO NOT include any YAML frontmatter. Start immediately with the markdown content (e.g. headers). - -**Guidelines:** -1. **Ignore Noise**: Exclude navigation bars, footers, "Edit this page" links, and other non-technical content. -2. **Decision Trees**: If the content describes a process with multiple choices or steps, YOU MUST create a "Decision Logic" or "Flowchart" section to guide the agent. -3. **Clarity**: Use clear headings, bullet points, and code blocks. -4. **Format**: Do NOT wrap the entire output in a markdown code block (like ```markdown ... ```). Return raw markdown text. -${instructions != null && instructions.isNotEmpty ? '5. **Special Instructions**: $instructions' : ''} - -Raw Content: -$markdown -'''; - } - GenerateContentRequest _createRequest( String prompt, { String? systemInstruction, @@ -282,65 +244,3 @@ class _ApiKeyClient extends http.BaseClient { return _inner.send(request); } } - -/// Fetches and converts content from a list of resources. -/// -/// Throws an [Exception] if fetching any resource fails. This strict behavior -/// prevents wasting Gemini tokens on generating low-quality skills when -/// source material is missing. -Future fetchAndConvertContent( - List resources, - http.Client httpClient, - Logger logger, { - io.Directory? configDir, -}) async { - final converter = MarkdownConverter(); - final sb = StringBuffer(); - for (final resource in resources) { - logger.info(' Fetching $resource...'); - - if (resource.startsWith('http://')) { - throw Exception( - 'Insecure HTTP URL found: $resource. ' - 'Only HTTPS URLs or relative file paths are allowed.', - ); - } - - if (resource.startsWith('https://')) { - final response = await httpClient.get(Uri.parse(resource)); - if (response.statusCode == 200) { - sb - ..writeln('--- Raw content from $resource ---') - ..writeln(converter.convert(response.body)); - } else { - throw Exception( - 'Failed to fetch $resource: HTTP ${response.statusCode}. ' - 'Failing fast to save Gemini tokens.', - ); - } - } else { - if (configDir == null) { - throw Exception( - 'Relative resource "$resource" found, but no configuration ' - 'directory was provided to resolve it.', - ); - } - final file = io.File(p.join(configDir.path, resource)); - if (!file.existsSync()) { - throw Exception('Local resource file not found: ${file.path}'); - } - - final String content; - try { - content = file.readAsStringSync(); - } on io.FileSystemException { - throw Exception('Local resource file is not readable: ${file.path}'); - } - - sb - ..writeln('--- Raw content from $resource ---') - ..writeln(content); - } - } - return sb.toString(); -} diff --git a/tool/lib/src/services/prompts.dart b/tool/lib/src/services/prompts.dart new file mode 100644 index 0000000..ae4aae6 --- /dev/null +++ b/tool/lib/src/services/prompts.dart @@ -0,0 +1,62 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// ignore_for_file: avoid_classes_with_only_static_members + +/// Manages prompts used by the GeminiService. +class Prompts { + /// Creates the prompt for generating a new skill. + static String createSkillPrompt(String markdown, String? instructions) { + return ''' +Rewrite the following technical documentation into a high-quality "SKILL.md" file. + +DO NOT include any YAML frontmatter. Start immediately with the markdown content (e.g. headers). + +**Guidelines:** +1. **Ignore Noise**: Exclude navigation bars, footers, "Edit this page" links, and other non-technical content. +2. **Decision Trees**: If the content describes a process with multiple choices or steps, YOU MUST create a "Decision Logic" or "Flowchart" section to guide the agent. +3. **Clarity**: Use clear headings, bullet points, and code blocks. +4. **Format**: Do NOT wrap the entire output in a markdown code block (like ```markdown ... ```). Return raw markdown text. +${instructions != null && instructions.isNotEmpty ? '5. **Special Instructions**: $instructions' : ''} + +Raw Content: +$markdown +'''; + } + + /// Creates the prompt for validating an existing skill. + static String validateExistingSkillContentPrompt( + String markdown, + String instructions, + String generationDate, + String modelName, + String currentSkillContent, + ) { + return ''' +Validate the following skill document against the provided source material and verify if it is valid. +Focus on: +1. Accuracy: Does the skill capture the technical details correctly based on the Source Material? +2. Structure: Is the skill well-structured according to skill best practices? +3. Completeness: Is any critical information missing in the skill that is present in the Source Material? + +Context: +- The skill was originally generated on: $generationDate +- The current evaluation is using model: $modelName +- The instructions used to generate the skill were: +$instructions + +Source Material: +$markdown + +Current Skill Content: + "$currentSkillContent" +--- + +Grade the current output based on the instructions and the comparison to current website content and instructions today. +Establish a conclusion on whether the new skill is valid or not. +Reasons for a good or bad quality grade should be provided including concepts such as missing content, different model used, more than a few months old, etc. +On the very last line, output "Grade: [0-100]" representing overall quality of the skill compared to the assumed value if it were generated again today. +'''; + } +} diff --git a/tool/lib/src/services/resource_fetcher_service.dart b/tool/lib/src/services/resource_fetcher_service.dart new file mode 100644 index 0000000..3b5aa18 --- /dev/null +++ b/tool/lib/src/services/resource_fetcher_service.dart @@ -0,0 +1,84 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:io' as io; + +import 'package:http/http.dart' as http; +import 'package:logging/logging.dart'; +import 'package:path/path.dart' as p; + +import 'markdown_converter.dart'; + +/// Fetches and converts content from diverse resources. +class ResourceFetcherService { + /// Creates a new [ResourceFetcherService]. + ResourceFetcherService({ + required http.Client httpClient, + required Logger logger, + }) : _httpClient = httpClient, + _logger = logger; + + final http.Client _httpClient; + final Logger _logger; + + /// Fetches and converts content from a list of resources. + /// + /// Throws an [Exception] if fetching any resource fails. This strict behavior + /// prevents wasting tokens on generating low-quality skills when + /// source material is missing. + Future fetchAndConvertContent( + List resources, { + io.Directory? configDir, + }) async { + final converter = MarkdownConverter(); + final sb = StringBuffer(); + for (final resource in resources) { + _logger.info(' Fetching $resource...'); + + if (resource.startsWith('http://')) { + throw Exception( + 'Insecure HTTP URL found: $resource. ' + 'Only HTTPS URLs or relative file paths are allowed.', + ); + } + + if (resource.startsWith('https://')) { + final response = await _httpClient.get(Uri.parse(resource)); + if (response.statusCode == 200) { + sb + ..writeln('--- Raw content from $resource ---') + ..writeln(converter.convert(response.body)); + } else { + throw Exception( + 'Failed to fetch $resource: HTTP ${response.statusCode}. ' + 'Failing fast to save Gemini tokens.', + ); + } + } else { + if (configDir == null) { + throw Exception( + 'Relative resource "$resource" found, but no configuration ' + 'directory was provided to resolve it.', + ); + } + final file = io.File(p.join(configDir.path, resource)); + if (!file.existsSync()) { + throw Exception('Local resource file not found: ${file.path}'); + } + + final String content; + try { + content = file.readAsStringSync(); + } on io.FileSystemException { + throw Exception('Local resource file is not readable: ${file.path}'); + } + + sb + ..writeln('--- Raw content from $resource ---') + ..writeln(content); + } + } + return sb.toString(); + } +} diff --git a/tool/test/commands/fetch_and_convert_content_test.dart b/tool/test/commands/fetch_and_convert_content_test.dart deleted file mode 100644 index 3ada709..0000000 --- a/tool/test/commands/fetch_and_convert_content_test.dart +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'package:http/http.dart' as http; -import 'package:http/testing.dart'; -import 'package:logging/logging.dart'; -import 'package:skills/src/services/gemini_service.dart'; - -import 'package:test/test.dart'; - -void main() { - group('fetchAndConvertContent', () { - late Logger logger; - late List logs; - - setUp(() { - logger = Logger('test'); - logs = []; - Logger.root.onRecord.listen((record) { - logs.add(record.message); - }); - }); - - test('fetches and converts content successfully', () async { - final client = MockClient((request) async { - if (request.url.toString() == 'https://example.com') { - return http.Response('

Hello

', 200); - } - return http.Response('Not Found', 404); - }); - - final result = await fetchAndConvertContent( - ['https://example.com'], - client, - logger, - ); - - expect(result, contains('--- Raw content from https://example.com ---')); - expect(result, contains('# Hello')); - }); - - test('handles failed fetch', () async { - final client = MockClient((request) async { - return http.Response('Not Found', 404); - }); - - expect( - () => fetchAndConvertContent(['https://example.com'], client, logger), - throwsA(isA()), - ); - }); - - test('handles exception during fetch', () async { - final client = MockClient((request) async { - throw Exception('Network error'); - }); - - expect( - () => fetchAndConvertContent(['https://example.com'], client, logger), - throwsA(isA()), - ); - }); - }); -} diff --git a/tool/test/services/gemini_service_test.dart b/tool/test/services/gemini_service_test.dart index b133498..e3a32ca 100644 --- a/tool/test/services/gemini_service_test.dart +++ b/tool/test/services/gemini_service_test.dart @@ -2,12 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import 'dart:io' as io; - import 'package:http/http.dart' as http; -import 'package:http/testing.dart'; -import 'package:logging/logging.dart'; -import 'package:path/path.dart' as p; import 'package:skills/src/services/gemini_service.dart'; import 'package:test/test.dart'; @@ -193,161 +188,4 @@ Some content }); }); }); - - group('fetchAndConvertContent', () { - late Logger logger; - - setUp(() { - logger = Logger('test'); - }); - - test('fetches and converts content successfully on 200 OK', () async { - final client = MockClient((request) async { - if (request.url.toString() == 'https://example.com/doc1') { - return http.Response('

Doc 1

', 200); - } else if (request.url.toString() == 'https://example.com/doc2') { - return http.Response('

Doc 2 content

', 200); - } - return http.Response('Not found', 404); - }); - - final result = await fetchAndConvertContent( - ['https://example.com/doc1', 'https://example.com/doc2'], - client, - logger, - ); - - expect(result, contains('Doc 1')); - expect(result, contains('Doc 2 content')); - }); - - test( - 'throws Exception on non-200 status code to save Gemini tokens', - () async { - final client = MockClient((request) async { - return http.Response('Not found', 404); - }); - - expect( - () => fetchAndConvertContent( - ['https://example.com/missing'], - client, - logger, - ), - throwsA( - isA().having( - (e) => e.toString(), - 'message', - contains('HTTP 404'), - ), - ), - ); - }, - ); - - test('throws exception on network error to save Gemini tokens', () async { - final client = MockClient((request) async { - throw http.ClientException('Connection failed'); - }); - - expect( - () => fetchAndConvertContent( - ['https://example.com/error'], - client, - logger, - ), - throwsA(isA()), - ); - }); - - test('throws Exception for insecure http:// URL', () async { - final client = MockClient((request) async { - return http.Response('content', 200); - }); - - expect( - () => - fetchAndConvertContent(['http://example.com/doc1'], client, logger), - throwsA( - isA().having( - (e) => e.toString(), - 'message', - contains('Insecure HTTP URL found'), - ), - ), - ); - }); - - test('fetches local file correctly relative to configDir', () async { - final tempDir = io.Directory.systemTemp.createTempSync('gemini_test'); - try { - io.File( - p.join(tempDir.path, 'local_doc.md'), - ).writeAsStringSync('# Local Doc\ncontent'); - - final client = MockClient((request) async { - return http.Response('Not found', 404); - }); - - final result = await fetchAndConvertContent( - ['local_doc.md'], - client, - logger, - configDir: tempDir, - ); - - expect(result, contains('Local Doc')); - expect(result, contains('content')); - } finally { - tempDir.deleteSync(recursive: true); - } - }); - - test('throws Exception for missing local file', () async { - final tempDir = io.Directory.systemTemp.createTempSync('gemini_test'); - try { - final client = MockClient((request) async { - return http.Response('Not found', 404); - }); - - expect( - () => fetchAndConvertContent( - ['missing.md'], - client, - logger, - configDir: tempDir, - ), - throwsA( - isA().having( - (e) => e.toString(), - 'message', - contains('Local resource file not found'), - ), - ), - ); - } finally { - tempDir.deleteSync(recursive: true); - } - }); - - test( - 'throws Exception for local file when no configDir is provided', - () async { - final client = MockClient((request) async { - return http.Response('Not found', 404); - }); - - expect( - () => fetchAndConvertContent(['local_doc.md'], client, logger), - throwsA( - isA().having( - (e) => e.toString(), - 'message', - contains('no configuration directory was provided to resolve it'), - ), - ), - ); - }, - ); - }); } diff --git a/tool/test/services/resource_fetcher_service_test.dart b/tool/test/services/resource_fetcher_service_test.dart new file mode 100644 index 0000000..8d4f43f --- /dev/null +++ b/tool/test/services/resource_fetcher_service_test.dart @@ -0,0 +1,188 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:io' as io; + +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:logging/logging.dart'; +import 'package:path/path.dart' as p; + +import 'package:skills/src/services/resource_fetcher_service.dart'; +import 'package:test/test.dart'; + +void main() { + group('ResourceFetcherService', () { + late Logger logger; + + setUp(() { + logger = Logger('test'); + }); + + test('fetches and converts content successfully on 200 OK', () async { + final client = MockClient((request) async { + if (request.url.toString() == 'https://example.com/doc1') { + return http.Response('

Doc 1

', 200); + } else if (request.url.toString() == 'https://example.com/doc2') { + return http.Response('

Doc 2 content

', 200); + } + return http.Response('Not found', 404); + }); + + final fetcher = ResourceFetcherService( + httpClient: client, + logger: logger, + ); + + final result = await fetcher.fetchAndConvertContent([ + 'https://example.com/doc1', + 'https://example.com/doc2', + ]); + + expect(result, contains('Doc 1')); + expect(result, contains('Doc 2 content')); + }); + + test('throws Exception on non-200 status code to save tokens', () async { + final client = MockClient((request) async { + return http.Response('Not found', 404); + }); + + final fetcher = ResourceFetcherService( + httpClient: client, + logger: logger, + ); + + expect( + () => fetcher.fetchAndConvertContent(['https://example.com/missing']), + throwsA( + isA().having( + (e) => e.toString(), + 'message', + contains('HTTP 404'), + ), + ), + ); + }); + + test('throws exception on network error to save tokens', () async { + final client = MockClient((request) async { + throw http.ClientException('Connection failed'); + }); + + final fetcher = ResourceFetcherService( + httpClient: client, + logger: logger, + ); + + expect( + () => fetcher.fetchAndConvertContent(['https://example.com/error']), + throwsA(isA()), + ); + }); + + test('throws Exception for insecure http:// URL', () async { + final client = MockClient((request) async { + return http.Response('content', 200); + }); + + final fetcher = ResourceFetcherService( + httpClient: client, + logger: logger, + ); + + expect( + () => fetcher.fetchAndConvertContent(['http://example.com/doc1']), + throwsA( + isA().having( + (e) => e.toString(), + 'message', + contains('Insecure HTTP URL found'), + ), + ), + ); + }); + + test('fetches local file correctly relative to configDir', () async { + final tempDir = io.Directory.systemTemp.createTempSync('gemini_test'); + try { + io.File( + p.join(tempDir.path, 'local_doc.md'), + ).writeAsStringSync('# Local Doc\ncontent'); + + final client = MockClient((request) async { + return http.Response('Not found', 404); + }); + + final fetcher = ResourceFetcherService( + httpClient: client, + logger: logger, + ); + + final result = await fetcher.fetchAndConvertContent([ + 'local_doc.md', + ], configDir: tempDir); + + expect(result, contains('Local Doc')); + expect(result, contains('content')); + } finally { + tempDir.deleteSync(recursive: true); + } + }); + + test('throws Exception for missing local file', () async { + final tempDir = io.Directory.systemTemp.createTempSync('gemini_test'); + try { + final client = MockClient((request) async { + return http.Response('Not found', 404); + }); + + final fetcher = ResourceFetcherService( + httpClient: client, + logger: logger, + ); + + expect( + () => fetcher.fetchAndConvertContent([ + 'missing.md', + ], configDir: tempDir), + throwsA( + isA().having( + (e) => e.toString(), + 'message', + contains('Local resource file not found'), + ), + ), + ); + } finally { + tempDir.deleteSync(recursive: true); + } + }); + + test( + 'throws Exception for local file when no configDir is provided', + () async { + final client = MockClient((request) async { + return http.Response('Not found', 404); + }); + + final fetcher = ResourceFetcherService( + httpClient: client, + logger: logger, + ); + + expect( + () => fetcher.fetchAndConvertContent(['local_doc.md']), + throwsA( + isA().having( + (e) => e.toString(), + 'message', + contains('no configuration directory was provided to resolve it'), + ), + ), + ); + }, + ); + }); +}