From caaa57f0e05c92fc832956eb51976c541a4153f5 Mon Sep 17 00:00:00 2001 From: Jaemin Jo <44039707+91jaeminjo@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:43:22 -0500 Subject: [PATCH 1/2] refactor: shorten lobby sort labels and key the section headers Rename the lobby sort options to "Recent" and "Unread" and give the recency/unread section headers a stable ValueKey so tests can target a header independently of the identically-labelled sort option. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/src/modules/lobby/ui/lobby_screen.dart | 11 +++---- test/modules/lobby/ui/lobby_screen_test.dart | 31 +++++++++++++------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/lib/src/modules/lobby/ui/lobby_screen.dart b/lib/src/modules/lobby/ui/lobby_screen.dart index d2f4afe7..34c3acd8 100644 --- a/lib/src/modules/lobby/ui/lobby_screen.dart +++ b/lib/src/modules/lobby/ui/lobby_screen.dart @@ -800,8 +800,8 @@ class _ViewModeToggle extends StatelessWidget { /// labelled dropdown and the phone [_CompactSortButton] so the two can't drift. const _sortOptions = <(LobbySortMode, String)>[ (LobbySortMode.none, 'None'), - (LobbySortMode.recentActivity, 'Recent activity'), - (LobbySortMode.unreadFirst, 'Unread first'), + (LobbySortMode.recentActivity, 'Recent'), + (LobbySortMode.unreadFirst, 'Unread'), ]; /// Compact sort control for phone layouts: an icon button that opens a menu of @@ -990,11 +990,11 @@ class _ServerSection extends StatelessWidget { crossAxisAlignment: CrossAxisAlignment.start, children: [ if (parts.unread.isNotEmpty) ...[ - const _GroupHeader(label: 'Unread'), + _GroupHeader(label: 'Unread'), _buildBlock(context, parts.unread), ], if (parts.read.isNotEmpty) ...[ - const _GroupHeader(label: 'Read'), + _GroupHeader(label: 'Read'), _buildBlock(context, parts.read), ], ], @@ -1142,7 +1142,8 @@ class _RoomGrid extends StatelessWidget { /// A recency-bucket section header: a muted label with a trailing rule, used /// to separate "Today", "Yesterday", ... groups when sorting by activity. class _GroupHeader extends StatelessWidget { - const _GroupHeader({required this.label}); + _GroupHeader({required this.label}) + : super(key: ValueKey('lobby-section-header-$label')); final String label; diff --git a/test/modules/lobby/ui/lobby_screen_test.dart b/test/modules/lobby/ui/lobby_screen_test.dart index 4a2a8b1e..d1b10e1a 100644 --- a/test/modules/lobby/ui/lobby_screen_test.dart +++ b/test/modules/lobby/ui/lobby_screen_test.dart @@ -513,13 +513,13 @@ void main() { // Default sort (none): backend order is preserved. expect(roomOrder(), ['General', 'Random']); - // Open the compact menu and choose "Recent activity". + // Open the compact menu and choose "Recent". await tester.tap(find.byIcon(Icons.sort)); await tester.pumpAndSettle(); await tester.tap( find.widgetWithText( CheckedPopupMenuItem, - 'Recent activity', + 'Recent', ), ); await tester.pumpAndSettle(); @@ -651,12 +651,12 @@ void main() { // Default sort (none): backend order is preserved. expect(roomOrder(), ['General', 'Random']); - // Open the sort dropdown and choose "Recent activity" (the offstage + // Open the sort dropdown and choose "Recent" (the offstage // measurement copy of the entry isn't hit-testable, so target the // on-screen one). await tester.tap(find.byType(DropdownMenu)); await tester.pumpAndSettle(); - await tester.tap(find.text('Recent activity').hitTestable()); + await tester.tap(find.text('Recent').hitTestable()); await tester.pumpAndSettle(); expect(roomOrder(), ['Random', 'General']); @@ -718,7 +718,7 @@ void main() { await tester.tap(find.byType(DropdownMenu)); await tester.pumpAndSettle(); - await tester.tap(find.text('Recent activity').hitTestable()); + await tester.tap(find.text('Recent').hitTestable()); await tester.pumpAndSettle(); // Two buckets: today (Fresh) and ~10 days ago (This month). @@ -767,7 +767,7 @@ void main() { await tester.tap(find.byType(DropdownMenu)); await tester.pumpAndSettle(); - await tester.tap(find.text('Recent activity').hitTestable()); + await tester.tap(find.text('Recent').hitTestable()); await tester.pumpAndSettle(); // Dated rooms first (newest -> oldest), then undated in original order. @@ -803,18 +803,27 @@ void main() { await tester.pumpWidget(_buildApp(manager, apiResolver: (_) => fakeApi)); await tester.pumpAndSettle(); + // The sort control's own "Unread" option shares the section-header + // label, so target the header by its key rather than its text. + final unreadHeader = find.byKey( + const ValueKey('lobby-section-header-Unread'), + ); + final readHeader = find.byKey( + const ValueKey('lobby-section-header-Read'), + ); + // No section headers under the default 'None' sort. - expect(find.text('Unread'), findsNothing); - expect(find.text('Read'), findsNothing); + expect(unreadHeader, findsNothing); + expect(readHeader, findsNothing); await tester.tap(find.byType(DropdownMenu)); await tester.pumpAndSettle(); - await tester.tap(find.text('Unread first').hitTestable()); + await tester.tap(find.text('Unread').hitTestable()); await tester.pumpAndSettle(); // Both section headers present. - expect(find.text('Unread'), findsOneWidget); - expect(find.text('Read'), findsOneWidget); + expect(unreadHeader, findsOneWidget); + expect(readHeader, findsOneWidget); // The unread room card sits above the read room card. final freshY = tester.getTopLeft(find.text('Fresh')).dy; From 18fb2b7de6f7cf8b23a2ab5e2a318dee7bb49adb Mon Sep 17 00:00:00 2001 From: Jaemin Jo <44039707+91jaeminjo@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:34:59 -0500 Subject: [PATCH 2/2] feat: derive a document source link, with a consumer resolver fallback Prefer the backend viewer source_url for a document's source link. When it is absent, derive the citation link from the document URI via a consumer-injected resolver; otherwise show the URI as non-clickable text (it is never itself launchable). Expose the resolver as a seam through standard(documentBrowserUrl:) / standardFlavor(...), installed as a documentBrowserUrlResolverProvider override by a module, with the DocumentBrowserUrlResolver type on the public API. Validate resolver output as an http/https URL and guard the call so a throwing resolver degrades to no link. Log a malformed backend source_url at the citation and document parse sites, and log an http(s) launch failure as an error. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 10 ++- lib/soliplex_frontend.dart | 4 + lib/src/flavors/standard.dart | 6 ++ .../modules/room/document_browser_url.dart | 81 +++++++++++++++++++ .../modules/room/ui/citations_section.dart | 27 +++++-- lib/src/modules/room/ui/document_picker.dart | 9 +-- lib/src/modules/room/ui/document_source.dart | 43 ++++++++++ .../room/ui/room_info/documents_card.dart | 13 ++- .../shared/markdown/launch_markdown_link.dart | 31 ++++--- .../soliplex_client/lib/src/api/mappers.dart | 11 ++- .../src/application/citation_extractor.dart | 10 +++ .../lib/src/utils/source_url.dart | 31 +++++-- .../soliplex_client/lib/src/utils/utils.dart | 1 + test/flavors/standard_flavor_test.dart | 28 +++++++ .../room/document_browser_url_test.dart | 73 +++++++++++++++++ .../ui/citations_section_source_url_test.dart | 41 +++++++--- .../ui/document_picker_source_url_test.dart | 6 +- .../modules/room/ui/document_source_test.dart | 35 ++++++++ .../documents_card_source_url_test.dart | 4 +- .../ui/room_info/documents_card_test.dart | 8 +- 20 files changed, 407 insertions(+), 65 deletions(-) create mode 100644 lib/src/modules/room/document_browser_url.dart create mode 100644 lib/src/modules/room/ui/document_source.dart create mode 100644 test/modules/room/document_browser_url_test.dart create mode 100644 test/modules/room/ui/document_source_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index babb575f..ae2421dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,12 @@ Versions follow the `version+build` scheme from `pubspec.yaml`, bumped via ### Added -- Document origin URLs (`source_url`) now render as clickable links across the - room document listing, document filter, and citations, replacing the internal - file path — which remains only in the document listing's metadata dialog. +- A document's source now shows as a clickable link when the backend provides a + viewer `source_url`, across the room document listing, document filter, and + citations. When `source_url` is absent, the citation link can be derived from + the document URI by a resolver a deployment injects via + `standard(documentBrowserUrl: ...)`; otherwise the document URI is shown as + non-clickable text. - Room and lobby: the current server's name (or its address when unnamed) now shows alongside the room name in the room view header, and as a title band at the top of the lobby's room pane, so a user connected to several servers can @@ -58,6 +61,7 @@ Versions follow the `version+build` scheme from `pubspec.yaml`, bumped via danger-styled text button, so the safe choice carries the emphasis. - In an expanded citation, the cited figures now appear above the source text rather than below it. +- The lobby sort options are shortened to "None", "Recent", and "Unread". ### Fixed diff --git a/lib/soliplex_frontend.dart b/lib/soliplex_frontend.dart index c6fa8a16..45048b64 100644 --- a/lib/soliplex_frontend.dart +++ b/lib/soliplex_frontend.dart @@ -35,6 +35,10 @@ export 'src/core/shell_config.dart' show ShellConfig; export 'src/core/status_message_config.dart' show StatusMessageConfig; export 'src/flavors/standard.dart' show standard, standardFlavor; export 'src/flavors/standard_kit.dart' show buildStandardKit, StandardKit; +// The resolver type for `standard(documentBrowserUrl: ...)`; the provider that +// installs it stays internal. +export 'src/modules/room/document_browser_url.dart' + show DocumentBrowserUrlResolver; export 'src/interfaces/auth_state.dart' show AuthState, Authenticated, Unauthenticated; export 'src/modules/auth/auth_providers.dart' diff --git a/lib/src/flavors/standard.dart b/lib/src/flavors/standard.dart index 596d6187..bb4876ec 100644 --- a/lib/src/flavors/standard.dart +++ b/lib/src/flavors/standard.dart @@ -9,6 +9,7 @@ import '../core/shell_config.dart'; import '../core/status_message_config.dart'; import '../modules/auth/consent_notice.dart'; import '../modules/auth/platform/callback_params.dart'; +import '../modules/room/document_browser_url.dart'; import 'standard_kit.dart'; /// Builds the standard [Flavor]: the full module set on shared session @@ -32,6 +33,7 @@ Future standardFlavor({ bool enableDocumentFilter = true, String statusMessageFilePath = StatusMessageConfig.defaultFilePath, Duration statusMessagePollInterval = StatusMessageConfig.defaultPollInterval, + DocumentBrowserUrlResolver? documentBrowserUrl, List Function(StandardKit kit)? extraModules, }) async { final effectiveIdentity = identity ?? AppIdentity.soliplex; @@ -52,6 +54,8 @@ Future standardFlavor({ theme: theme, modules: [ ...kit.modules, + if (documentBrowserUrl != null) + DocumentBrowserUrlModule(documentBrowserUrl), ...?extraModules?.call(kit), ], initialRoute: kit.initialRoute, @@ -80,6 +84,7 @@ Future standard({ Duration inactivityGraceDuration = InactivityConfig.defaultGraceDuration, String statusMessageFilePath = StatusMessageConfig.defaultFilePath, Duration statusMessagePollInterval = StatusMessageConfig.defaultPollInterval, + DocumentBrowserUrlResolver? documentBrowserUrl, }) async { final flavor = await standardFlavor( identity: identity, @@ -97,6 +102,7 @@ Future standard({ inactivityGraceDuration: inactivityGraceDuration, statusMessageFilePath: statusMessageFilePath, statusMessagePollInterval: statusMessagePollInterval, + documentBrowserUrl: documentBrowserUrl, ); return flavor.build(); } diff --git a/lib/src/modules/room/document_browser_url.dart b/lib/src/modules/room/document_browser_url.dart new file mode 100644 index 00000000..7adbc2c7 --- /dev/null +++ b/lib/src/modules/room/document_browser_url.dart @@ -0,0 +1,81 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:soliplex_client/soliplex_client.dart'; +import 'package:soliplex_logging/soliplex_logging.dart'; + +import '../../core/app_module.dart'; + +final _logger = + LogManager.instance.getLogger('soliplex_frontend.document_browser_url'); + +/// Derives a document's clickable browser URL from its internal document URI. +typedef DocumentBrowserUrlResolver = Uri? Function(String documentUri); + +/// A deployment's rule for turning an internal document URI into a public +/// browser URL, used as a fallback for the citation source link. +/// +/// The backend supplies a viewer `source_url` only for documents whose +/// ingestion attached it. When it is absent, the internal `documentUri` +/// (typically a `file://`/`s3://` path) is never itself launchable, so a +/// deployment can supply a rule that maps it to a public URL. The concrete +/// mapping lives in the consumer, injected through `standard(documentBrowserUrl: +/// ...)` / `standardFlavor(documentBrowserUrl: ...)`. The default resolves +/// nothing, so the standard build shows the raw document URI instead of a link. +/// +/// Only the citation source link consults this; the document listing and filter +/// read `RagDocument.sourceUrl` directly. +final documentBrowserUrlResolverProvider = + Provider((_) => (_) => null); + +/// The launchable browser URL for a cited document: the viewer [sourceUrl] when +/// present, else a URL the [resolver] derives from [documentUri]. The resolver +/// result is validated as a web URL, so a resolver that returns a +/// non-launchable value yields null rather than a dead link. +Uri? resolveDocumentBrowserUrl( + DocumentBrowserUrlResolver resolver, { + required Uri? sourceUrl, + required String documentUri, +}) { + if (sourceUrl != null) return sourceUrl; + + final Uri? derived; + try { + derived = resolver(documentUri); + } catch (error, stackTrace) { + // The resolver is consumer-supplied; a throw must degrade to no link, not + // crash the widget that renders it. + _logger.error( + 'documentBrowserUrl resolver threw', + error: error, + stackTrace: stackTrace, + ); + return null; + } + if (derived == null) return null; + + final url = launchableWebUrl(derived.toString()); + if (url == null) { + _logger.warning( + 'documentBrowserUrl resolver returned a non-launchable URL ' + '(scheme: ${derived.scheme})', + ); + } + return url; +} + +/// Installs [resolver] as the app-wide [documentBrowserUrlResolverProvider] +/// override. Added by `standardFlavor` when a `documentBrowserUrl` is supplied. +class DocumentBrowserUrlModule extends AppModule { + DocumentBrowserUrlModule(this.resolver); + + final DocumentBrowserUrlResolver resolver; + + @override + String get namespace => 'document_browser_url'; + + @override + ModuleRoutes build() => ModuleRoutes( + overrides: [ + documentBrowserUrlResolverProvider.overrideWithValue(resolver), + ], + ); +} diff --git a/lib/src/modules/room/ui/citations_section.dart b/lib/src/modules/room/ui/citations_section.dart index d78c7015..c772a6c8 100644 --- a/lib/src/modules/room/ui/citations_section.dart +++ b/lib/src/modules/room/ui/citations_section.dart @@ -1,14 +1,16 @@ import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:soliplex_agent/soliplex_agent.dart' hide State; import 'package:soliplex_design/soliplex_design.dart'; import 'package:soliplex_logging/soliplex_logging.dart'; -import '../../../shared/browser_url_link.dart'; import '../../../shared/copy_button.dart'; import '../../../shared/failed_image.dart'; import '../../../shared/preview_icon_button.dart'; import '../../../shared/zoomable_image.dart'; import '../../../shared/zoomable_view.dart'; +import '../document_browser_url.dart'; +import 'document_source.dart'; import 'markdown/flutter_markdown_plus_renderer.dart'; import 'markdown/log_source.dart'; import 'paged_zoomable_images.dart'; @@ -256,14 +258,23 @@ class _SourceReferenceRow extends StatelessWidget { child: Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - // The cited document's clickable origin link, when the backend - // shipped a usable `source_url`. The internal path is never shown - // (the row header already carries the document name). - if (sourceReference.sourceUrl case final url?) - Padding( - padding: const EdgeInsets.only(bottom: SoliplexSpacing.s1), - child: BrowserUrlLink(url: url), + // The cited document's source link: the viewer `source_url`, else a + // resolver-derived URL from the document URI, else the raw URI as + // text (it is never itself launchable). A fork supplies the resolver + // via documentBrowserUrlResolverProvider. + Padding( + padding: const EdgeInsets.only(bottom: SoliplexSpacing.s1), + child: Consumer( + builder: (context, ref, _) => DocumentSource( + url: resolveDocumentBrowserUrl( + ref.watch(documentBrowserUrlResolverProvider), + sourceUrl: sourceReference.sourceUrl, + documentUri: sourceReference.documentUri, + ), + documentUri: sourceReference.documentUri, + ), ), + ), if (sourceReference.figures.isNotEmpty) _CitationFigures(sourceReference: sourceReference), if (sourceReference.headings.isNotEmpty) ...[ diff --git a/lib/src/modules/room/ui/document_picker.dart b/lib/src/modules/room/ui/document_picker.dart index 0163c882..0445124f 100644 --- a/lib/src/modules/room/ui/document_picker.dart +++ b/lib/src/modules/room/ui/document_picker.dart @@ -3,8 +3,8 @@ import 'dart:developer' as developer; import 'package:flutter/material.dart'; import 'package:soliplex_client/soliplex_client.dart' hide State; -import '../../../shared/browser_url_link.dart'; import '../../../shared/file_type_icons.dart'; +import 'document_source.dart'; import 'package:soliplex_design/soliplex_design.dart'; class DocumentPicker extends StatefulWidget { @@ -43,11 +43,8 @@ class _DocumentPickerState extends State { widget.onChanged(next); } - Widget? _subtitle(RagDocument doc) { - final url = doc.sourceUrl; - if (url == null) return null; - return BrowserUrlLink(url: url); - } + Widget _subtitle(RagDocument doc) => + DocumentSource(url: doc.sourceUrl, documentUri: doc.uri); @override Widget build(BuildContext context) { diff --git a/lib/src/modules/room/ui/document_source.dart b/lib/src/modules/room/ui/document_source.dart new file mode 100644 index 00000000..1b449eab --- /dev/null +++ b/lib/src/modules/room/ui/document_source.dart @@ -0,0 +1,43 @@ +import 'package:flutter/material.dart'; + +import '../../../shared/browser_url_link.dart'; + +/// A document's source, shown in a link slot. +/// +/// Renders a clickable [BrowserUrlLink] for the launchable [url]; when there is +/// none, the [documentUri] is shown as muted, non-clickable text (the full +/// value on hover), since the URI is never itself launchable. Renders nothing +/// when neither is available. +class DocumentSource extends StatelessWidget { + const DocumentSource({ + required this.url, + required this.documentUri, + super.key, + }); + + /// The document's launchable browser URL, or null. + final Uri? url; + + /// The document's internal URI, shown as text when [url] is null. + final String documentUri; + + @override + Widget build(BuildContext context) { + final linkUrl = url; + if (linkUrl != null) return BrowserUrlLink(url: linkUrl); + + if (documentUri.isEmpty) return const SizedBox.shrink(); + final theme = Theme.of(context); + return Tooltip( + message: documentUri, + child: Text( + documentUri, + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: theme.textTheme.labelSmall?.copyWith( + color: theme.colorScheme.onSurfaceVariant, + ), + ), + ); + } +} diff --git a/lib/src/modules/room/ui/room_info/documents_card.dart b/lib/src/modules/room/ui/room_info/documents_card.dart index 3ded4926..81e45718 100644 --- a/lib/src/modules/room/ui/room_info/documents_card.dart +++ b/lib/src/modules/room/ui/room_info/documents_card.dart @@ -1,7 +1,7 @@ import 'package:flutter/material.dart'; import 'package:soliplex_client/soliplex_client.dart' hide State; -import '../../../../shared/browser_url_link.dart'; +import '../document_source.dart'; import '../../../../shared/file_type_icons.dart'; import 'room_info_widgets.dart'; import 'package:soliplex_design/soliplex_design.dart'; @@ -205,7 +205,6 @@ class _DocumentsCardState extends State { if (doc.updatedAt != null) { dateFields.add(('updated_at', _formatDateTime(doc.updatedAt!))); } - final sourceUrl = doc.sourceUrl; return Padding( padding: const EdgeInsets.only(top: SoliplexSpacing.s1), @@ -225,12 +224,10 @@ class _DocumentsCardState extends State { doc.id, style: context.monospaceOn(valueStyle), ), - if (sourceUrl != null) ...[ - const SizedBox(height: SoliplexSpacing.s2), - Text('link', style: labelStyle), - const SizedBox(height: SoliplexSpacing.s1), - BrowserUrlLink(url: sourceUrl), - ], + const SizedBox(height: SoliplexSpacing.s2), + Text('link', style: labelStyle), + const SizedBox(height: SoliplexSpacing.s1), + DocumentSource(url: doc.sourceUrl, documentUri: doc.uri), if (dateFields.isNotEmpty) ...[ const SizedBox(height: SoliplexSpacing.s2), Wrap( diff --git a/lib/src/shared/markdown/launch_markdown_link.dart b/lib/src/shared/markdown/launch_markdown_link.dart index 0b27af76..b6688ef1 100644 --- a/lib/src/shared/markdown/launch_markdown_link.dart +++ b/lib/src/shared/markdown/launch_markdown_link.dart @@ -7,9 +7,11 @@ final _logger = /// Opens a tapped markdown link in the platform's default handler — a browser /// for `http(s):`, the mail client for `mailto:`, and so on. /// -/// A missing handler (e.g. no mail client installed) is a normal failure, not -/// an error, so it is logged rather than surfaced. Only the scheme is logged; -/// the full href can carry an email address. +/// Failures are logged rather than surfaced. A missing handler for a +/// non-web scheme (e.g. no mail client for `mailto:`) is a normal, expected +/// failure logged at warning level; a failure on an `http(s)` link is a dead +/// link — every target platform has a browser — so it is logged as an error. +/// Only the scheme is logged; the full href can carry an email address. Future launchMarkdownLink(String href) async { final uri = Uri.tryParse(href); if (uri == null) { @@ -19,13 +21,24 @@ Future launchMarkdownLink(String href) async { try { final launched = await launchUrl(uri); if (!launched) { - _logger.warning('No handler available for link (scheme: ${uri.scheme})'); + _logLaunchFailure(uri, 'No handler available for link'); } } on Exception catch (error, stackTrace) { - _logger.warning( - 'Failed to launch link (scheme: ${uri.scheme})', - error: error, - stackTrace: stackTrace, - ); + _logLaunchFailure(uri, 'Failed to launch link', error, stackTrace); + } +} + +void _logLaunchFailure( + Uri uri, + String message, [ + Object? error, + StackTrace? stackTrace, +]) { + final detail = '$message (scheme: ${uri.scheme})'; + final isWeb = uri.scheme == 'http' || uri.scheme == 'https'; + if (isWeb) { + _logger.error(detail, error: error, stackTrace: stackTrace); + } else { + _logger.warning(detail, error: error, stackTrace: stackTrace); } } diff --git a/packages/soliplex_client/lib/src/api/mappers.dart b/packages/soliplex_client/lib/src/api/mappers.dart index c580f511..44113be0 100644 --- a/packages/soliplex_client/lib/src/api/mappers.dart +++ b/packages/soliplex_client/lib/src/api/mappers.dart @@ -14,6 +14,10 @@ import 'package:soliplex_client/src/domain/run_info.dart'; import 'package:soliplex_client/src/domain/thread_info.dart'; import 'package:soliplex_client/src/domain/workdir_file.dart'; import 'package:soliplex_client/src/utils/parse_utils.dart'; +import 'package:soliplex_client/src/utils/source_url.dart'; +import 'package:soliplex_logging/soliplex_logging.dart'; + +final _logger = LogManager.instance.getLogger('soliplex_client.mappers'); // ============================================================ // Timestamp helpers @@ -385,11 +389,16 @@ RagDocument ragDocumentFromJson(Map json) { final createdRaw = json['created_at'] as String?; final updatedRaw = json['updated_at'] as String?; + final metadata = jsonMap(json['metadata'], 'metadata'); + if (hasMalformedSourceUrl(metadata)) { + _logger.warning('Document source_url present but not a launchable web URL'); + } + return RagDocument( id: _requireString(json, 'id', 'document'), title: title, uri: uri, - metadata: jsonMap(json['metadata'], 'metadata'), + metadata: metadata, createdAt: _tryParseTimestamp(createdRaw), updatedAt: _tryParseTimestamp(updatedRaw), ); diff --git a/packages/soliplex_client/lib/src/application/citation_extractor.dart b/packages/soliplex_client/lib/src/application/citation_extractor.dart index 6ae9b04d..68683d23 100644 --- a/packages/soliplex_client/lib/src/application/citation_extractor.dart +++ b/packages/soliplex_client/lib/src/application/citation_extractor.dart @@ -2,6 +2,10 @@ import 'package:soliplex_client/src/application/rag_snapshot.dart'; import 'package:soliplex_client/src/domain/source_reference.dart'; import 'package:soliplex_client/src/schema/agui_features/rag.dart'; import 'package:soliplex_client/src/utils/source_url.dart'; +import 'package:soliplex_logging/soliplex_logging.dart'; + +final _logger = + LogManager.instance.getLogger('soliplex_client.citation_extractor'); /// Extracts new [SourceReference]s by comparing AG-UI state snapshots. /// @@ -78,6 +82,12 @@ class CitationExtractor { ), ); } + if (hasMalformedSourceUrl(c.documentMeta)) { + _logger.warning( + 'Citation source_url present but not a launchable web URL ' + '(document ${c.documentId})', + ); + } return SourceReference( documentId: c.documentId, documentUri: c.documentUri, diff --git a/packages/soliplex_client/lib/src/utils/source_url.dart b/packages/soliplex_client/lib/src/utils/source_url.dart index 8300f633..eda356b5 100644 --- a/packages/soliplex_client/lib/src/utils/source_url.dart +++ b/packages/soliplex_client/lib/src/utils/source_url.dart @@ -1,12 +1,10 @@ -/// Extracts a document's clickable origin URL from its free-form metadata. +/// Returns [value] as a browser-launchable web URL, or null. /// -/// Reads the `source_url` key and returns it only when it is a non-empty web -/// URL — `http`/`https` with a host. A raw `file://` path, the separate -/// `source_uri` upstream key, a hostless value, or a non-string all yield null. -/// `source_url` is the backend's viewer-ready URL; rejecting a non-web scheme -/// guards its contract so no blank-text link renders. -Uri? sourceUrlFromMetadata(Map metadata) { - final value = metadata['source_url']; +/// Accepts only a non-empty `http`/`https` string with a host. A raw +/// `file://`/`s3://` path, a hostless value, or a non-string all yield null — +/// so callers can hand the result straight to a link widget without rendering +/// a dead link. +Uri? launchableWebUrl(Object? value) { if (value is! String || value.isEmpty) return null; final uri = Uri.tryParse(value); if (uri == null || @@ -16,3 +14,20 @@ Uri? sourceUrlFromMetadata(Map metadata) { } return uri; } + +/// Extracts a document's clickable origin URL from its free-form metadata. +/// +/// Reads the `source_url` key — the backend's viewer-ready URL — and returns it +/// only when it is a launchable web URL (see [launchableWebUrl]). The separate +/// `source_uri` upstream key is deliberately ignored. +Uri? sourceUrlFromMetadata(Map metadata) => + launchableWebUrl(metadata['source_url']); + +/// Whether [metadata] carries a `source_url` that is present and non-empty but +/// not a launchable web URL — a malformed backend value, distinct from an +/// absent one. Lets callers log a backend contract drift while still degrading +/// gracefully to no link. +bool hasMalformedSourceUrl(Map metadata) { + final value = metadata['source_url']; + return value is String && value.isNotEmpty && launchableWebUrl(value) == null; +} diff --git a/packages/soliplex_client/lib/src/utils/utils.dart b/packages/soliplex_client/lib/src/utils/utils.dart index b2d5616f..3ecc9c9d 100644 --- a/packages/soliplex_client/lib/src/utils/utils.dart +++ b/packages/soliplex_client/lib/src/utils/utils.dart @@ -1,2 +1,3 @@ export 'cancel_token.dart'; +export 'source_url.dart'; export 'url_builder.dart'; diff --git a/test/flavors/standard_flavor_test.dart b/test/flavors/standard_flavor_test.dart index bcc9fb9d..4410c667 100644 --- a/test/flavors/standard_flavor_test.dart +++ b/test/flavors/standard_flavor_test.dart @@ -1,7 +1,9 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:soliplex_frontend/soliplex_frontend.dart'; import 'package:soliplex_frontend/src/core/routes.dart'; import 'package:soliplex_frontend/src/modules/auth/platform/callback_params.dart'; +import 'package:soliplex_frontend/src/modules/room/document_browser_url.dart'; import 'platform_mocks.dart'; @@ -40,4 +42,30 @@ void main() { expect(flavor.modules.sublist(0, kit.modules.length), kit.modules); expect(flavor.modules.last, same(extra)); }); + + test('documentBrowserUrl installs the resolver override', () async { + Uri? resolver(String uri) => Uri.parse('https://example.test/x'); + + final flavor = await standardFlavor( + callbackParams: WebCallbackSuccess(accessToken: 'x'), + documentBrowserUrl: resolver, + ); + final container = ProviderContainer(overrides: flavor.build().overrides); + addTearDown(container.dispose); + + expect(container.read(documentBrowserUrlResolverProvider), same(resolver)); + }); + + test('no documentBrowserUrl leaves the resolver returning null', () async { + final flavor = await standardFlavor( + callbackParams: WebCallbackSuccess(accessToken: 'x'), + ); + final container = ProviderContainer(overrides: flavor.build().overrides); + addTearDown(container.dispose); + + expect( + container.read(documentBrowserUrlResolverProvider)('file:///x/a.pdf'), + isNull, + ); + }); } diff --git a/test/modules/room/document_browser_url_test.dart b/test/modules/room/document_browser_url_test.dart new file mode 100644 index 00000000..96be0ee4 --- /dev/null +++ b/test/modules/room/document_browser_url_test.dart @@ -0,0 +1,73 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:soliplex_frontend/src/modules/room/document_browser_url.dart'; + +void main() { + test('the default resolver resolves nothing', () { + final container = ProviderContainer(); + addTearDown(container.dispose); + expect( + container.read(documentBrowserUrlResolverProvider)('file:///x/a.pdf'), + isNull, + ); + }); + + group('resolveDocumentBrowserUrl', () { + Uri? noResolve(String _) => null; + + test('prefers the source_url', () { + expect( + resolveDocumentBrowserUrl( + noResolve, + sourceUrl: Uri.parse('https://viewer.test/view'), + documentUri: 'file:///x/a.pdf', + ), + Uri.parse('https://viewer.test/view'), + ); + }); + + test('uses a resolver-derived web url when source_url is absent', () { + expect( + resolveDocumentBrowserUrl( + (_) => Uri.parse('https://viewer.test/derived'), + sourceUrl: null, + documentUri: 'file:///x/a.pdf', + ), + Uri.parse('https://viewer.test/derived'), + ); + }); + + test('is null when the resolver returns nothing', () { + expect( + resolveDocumentBrowserUrl( + noResolve, + sourceUrl: null, + documentUri: 'file:///x/a.pdf', + ), + isNull, + ); + }); + + test('rejects a resolver result that is not a web url', () { + expect( + resolveDocumentBrowserUrl( + (_) => Uri.parse('file:///nope.pdf'), + sourceUrl: null, + documentUri: 'file:///x/a.pdf', + ), + isNull, + ); + }); + + test('degrades to null when the resolver throws', () { + expect( + resolveDocumentBrowserUrl( + (_) => throw const FormatException('bad derived url'), + sourceUrl: null, + documentUri: 'file:///x/a.pdf', + ), + isNull, + ); + }); + }); +} diff --git a/test/modules/room/ui/citations_section_source_url_test.dart b/test/modules/room/ui/citations_section_source_url_test.dart index 2e0106dc..dae1b0ee 100644 --- a/test/modules/room/ui/citations_section_source_url_test.dart +++ b/test/modules/room/ui/citations_section_source_url_test.dart @@ -1,7 +1,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_riverpod/misc.dart' show Override; import 'package:flutter_test/flutter_test.dart'; import 'package:soliplex_agent/soliplex_agent.dart' hide State; +import 'package:soliplex_frontend/src/modules/room/document_browser_url.dart'; import 'package:soliplex_frontend/src/modules/room/ui/citations_section.dart'; import 'package:soliplex_frontend/src/shared/browser_url_link.dart'; @@ -15,8 +17,13 @@ SourceReference _ref({Uri? sourceUrl}) => SourceReference( index: 1, ); -Future _pump(WidgetTester tester, SourceReference ref) async { +Future _pump( + WidgetTester tester, + SourceReference ref, { + List overrides = const [], +}) async { await tester.pumpWidget(ProviderScope( + overrides: overrides, child: MaterialApp( home: Scaffold(body: CitationsSection(sourceReferences: [ref])), ), @@ -28,22 +35,32 @@ Future _pump(WidgetTester tester, SourceReference ref) async { } void main() { - testWidgets('no link and no raw path when the citation carries no source_url', + testWidgets('renders the link from the citation source_url', (tester) async { + final url = Uri.parse('https://example.test/foo.pdf/view'); + await _pump(tester, _ref(sourceUrl: url)); + expect(tester.widget(find.byType(BrowserUrlLink)).url, url); + }); + + testWidgets('a resolver derives the link when source_url is absent', (tester) async { - await _pump(tester, _ref()); - expect(find.byType(BrowserUrlLink), findsNothing); - // The internal file path is never shown. + final resolved = Uri.parse('https://viewer.test/foo.pdf/view'); + await _pump( + tester, + _ref(), + overrides: [ + documentBrowserUrlResolverProvider.overrideWithValue((_) => resolved), + ], + ); expect( - find.textContaining('file:///x/foo.pdf', findRichText: true), - findsNothing, + tester.widget(find.byType(BrowserUrlLink)).url, + resolved, ); }); - testWidgets('renders the link when the citation carries a source_url', + testWidgets('shows the document uri as text with no source_url or resolver', (tester) async { - final url = Uri.parse('https://example.test/foo.pdf/view'); - await _pump(tester, _ref(sourceUrl: url)); - final link = tester.widget(find.byType(BrowserUrlLink)); - expect(link.url, url); + await _pump(tester, _ref()); + expect(find.byType(BrowserUrlLink), findsNothing); + expect(find.text('file:///x/foo.pdf'), findsOneWidget); }); } diff --git a/test/modules/room/ui/document_picker_source_url_test.dart b/test/modules/room/ui/document_picker_source_url_test.dart index b38527ca..28e48307 100644 --- a/test/modules/room/ui/document_picker_source_url_test.dart +++ b/test/modules/room/ui/document_picker_source_url_test.dart @@ -23,17 +23,15 @@ void main() { ])); expect(find.byType(BrowserUrlLink), findsOneWidget); - expect(find.text('file:///files/Report.pdf'), findsNothing); }); - testWidgets('no subtitle link or raw uri when source_url absent', + testWidgets('subtitle shows the document uri when source_url absent', (tester) async { await tester.pumpWidget(_picker([ const RagDocument(id: '1', title: 'Report.pdf', uri: '/files/Report.pdf'), ])); expect(find.byType(BrowserUrlLink), findsNothing); - // The internal path is not shown in the picker. - expect(find.text('/files/Report.pdf'), findsNothing); + expect(find.text('/files/Report.pdf'), findsOneWidget); }); } diff --git a/test/modules/room/ui/document_source_test.dart b/test/modules/room/ui/document_source_test.dart new file mode 100644 index 00000000..a00fe9a3 --- /dev/null +++ b/test/modules/room/ui/document_source_test.dart @@ -0,0 +1,35 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:soliplex_frontend/src/modules/room/ui/document_source.dart'; +import 'package:soliplex_frontend/src/shared/browser_url_link.dart'; + +Widget _host(Widget child) => MaterialApp(home: Scaffold(body: child)); + +void main() { + testWidgets('renders a browser link when a url is given', (tester) async { + final url = Uri.parse('https://viewer.test/view'); + await tester.pumpWidget( + _host(DocumentSource(url: url, documentUri: 'file:///x/a.pdf')), + ); + expect(tester.widget(find.byType(BrowserUrlLink)).url, url); + expect(find.text('file:///x/a.pdf'), findsNothing); + }); + + testWidgets('shows the document uri as text when there is no url', + (tester) async { + await tester.pumpWidget( + _host(const DocumentSource(url: null, documentUri: 'file:///x/a.pdf')), + ); + expect(find.byType(BrowserUrlLink), findsNothing); + expect(find.text('file:///x/a.pdf'), findsOneWidget); + }); + + testWidgets('renders nothing when there is no url and no document uri', + (tester) async { + await tester.pumpWidget( + _host(const DocumentSource(url: null, documentUri: '')), + ); + expect(find.byType(BrowserUrlLink), findsNothing); + expect(find.byType(Text), findsNothing); + }); +} diff --git a/test/modules/room/ui/room_info/documents_card_source_url_test.dart b/test/modules/room/ui/room_info/documents_card_source_url_test.dart index 1ccaa5d7..809d961d 100644 --- a/test/modules/room/ui/room_info/documents_card_source_url_test.dart +++ b/test/modules/room/ui/room_info/documents_card_source_url_test.dart @@ -34,7 +34,7 @@ void main() { expect(find.text('file:///x/foo.pdf'), findsNothing); }); - testWidgets('no link and no raw uri in detail when source_url absent', + testWidgets('detail shows the document uri when source_url absent', (tester) async { await tester.pumpWidget(_card([ const RagDocument(id: 'd1', title: 'Doc', uri: 'file:///x/foo.pdf'), @@ -45,7 +45,7 @@ void main() { await tester.pumpAndSettle(); expect(find.byType(BrowserUrlLink), findsNothing); - expect(find.text('file:///x/foo.pdf'), findsNothing); + expect(find.text('file:///x/foo.pdf'), findsOneWidget); }); testWidgets('metadata dialog carries the file uri and a friendly title', diff --git a/test/modules/room/ui/room_info/documents_card_test.dart b/test/modules/room/ui/room_info/documents_card_test.dart index db8f4863..9bb18645 100644 --- a/test/modules/room/ui/room_info/documents_card_test.dart +++ b/test/modules/room/ui/room_info/documents_card_test.dart @@ -227,7 +227,7 @@ void main() { expect(find.text('id'), findsNothing); }); - testWidgets('metadata shows document ID but not the internal uri', + testWidgets('metadata shows document ID and the document uri as the source', (tester) async { await tester.pumpWidget(wrap( DocumentsCard( @@ -241,9 +241,9 @@ void main() { await tester.pump(); expect(find.text('id-a'), findsOneWidget); - // The internal path is not surfaced in the expanded detail (it lives in - // the metadata dialog). - expect(find.text('/files/alpha.pdf'), findsNothing); + // With no source_url, the document uri is shown (non-clickable) as the + // document's source. + expect(find.text('/files/alpha.pdf'), findsOneWidget); }); testWidgets('metadata shows timestamps when present', (tester) async {