From becb6c78b4136f436f34150fd0e35c126cc72598 Mon Sep 17 00:00:00 2001 From: TyperBody Date: Sat, 26 Sep 2026 03:51:36 +0800 Subject: [PATCH] fix(operation-trace): satisfy ruff/codeql and drop a dead summary channel - Remove the unused request_summary local: the stored summary is derived by the service from changes + the classified action, so handlers publishing quart.g.operation_log_summary was a dead channel that never reached the row. - Drop the unused datetime import in the operation log model. - Apply ruff format to the touched files so `ruff format --check` passes. --- src/langbot/pkg/api/http/controller/group.py | 3 +- .../api/http/controller/groups/settings.py | 2 +- .../api/http/controller/groups/workspaces.py | 2 +- src/langbot/pkg/api/http/service/settings.py | 71 +++++++++++-------- src/langbot/pkg/core/app.py | 4 +- .../pkg/entity/persistence/operation_log.py | 6 +- 6 files changed, 51 insertions(+), 37 deletions(-) diff --git a/src/langbot/pkg/api/http/controller/group.py b/src/langbot/pkg/api/http/controller/group.py index 079020893..19556bb21 100644 --- a/src/langbot/pkg/api/http/controller/group.py +++ b/src/langbot/pkg/api/http/controller/group.py @@ -413,12 +413,13 @@ class RouterGroup(abc.ABC): # static route declaration cannot describe the runtime diff, e.g. # the previous and the new role of a member. ``quart.g`` is # request-local, so concurrent requests never share these values. + # The stored summary is derived by the service from ``changes`` and + # the classified action, so handlers only publish the diff itself. changes = resolved_meta.get('changes') detail = resolved_meta.get('detail') request_changes = getattr(quart.g, 'operation_log_changes', None) request_detail = getattr(quart.g, 'operation_log_detail', None) request_resource_id = getattr(quart.g, 'operation_log_resource_id', None) - request_summary = getattr(quart.g, 'operation_log_summary', None) if request_changes: changes = request_changes if request_detail: diff --git a/src/langbot/pkg/api/http/controller/groups/settings.py b/src/langbot/pkg/api/http/controller/groups/settings.py index ee906859b..515d042e6 100644 --- a/src/langbot/pkg/api/http/controller/groups/settings.py +++ b/src/langbot/pkg/api/http/controller/groups/settings.py @@ -244,7 +244,7 @@ class SettingsRouterGroup(group.RouterGroup): # Prepend a UTF-8 BOM so spreadsheet tools detect the encoding. body = '\ufeff' + '\r\n'.join(lines) + '\r\n' - filename = f"operation-logs-{document['exported_at'].replace(':', '')}.csv" + filename = f'operation-logs-{document["exported_at"].replace(":", "")}.csv' response = quart.Response(body, mimetype='text/csv', content_type='text/csv; charset=utf-8') response.headers['Content-Disposition'] = f'attachment; filename="{filename}"' return response diff --git a/src/langbot/pkg/api/http/controller/groups/workspaces.py b/src/langbot/pkg/api/http/controller/groups/workspaces.py index 9b3f2ac99..b8b6d140f 100644 --- a/src/langbot/pkg/api/http/controller/groups/workspaces.py +++ b/src/langbot/pkg/api/http/controller/groups/workspaces.py @@ -224,7 +224,7 @@ class WorkspacesRouterGroup(group.RouterGroup): { 'field': 'invitation', 'before': None, - 'after': f"{created.invitation.normalized_email}:{created.invitation.role}", + 'after': f'{created.invitation.normalized_email}:{created.invitation.role}', } ] quart.g.operation_log_changes = invite_changes diff --git a/src/langbot/pkg/api/http/service/settings.py b/src/langbot/pkg/api/http/service/settings.py index a9c541b47..1876f62e8 100644 --- a/src/langbot/pkg/api/http/service/settings.py +++ b/src/langbot/pkg/api/http/service/settings.py @@ -651,7 +651,7 @@ def build_summary(rule: ActionRule, changes: list[dict[str, typing.Any]]) -> str return '' fragments: list[str] = [] for change in changes[:_MAX_SUMMARY_FIELDS]: - fragments.append(f"{change['field']}: {change['before']} → {change['after']}") + fragments.append(f'{change["field"]}: {change["before"]} → {change["after"]}') remaining = len(changes) - len(fragments) if remaining > 0: fragments.append(f'+{remaining}') @@ -1287,9 +1287,11 @@ class WorkspaceSettingsService: level: int | None = None, ) -> int: try: - query = sqlalchemy.select(sqlalchemy.func.count()).select_from( - persistence_operation_log.WorkspaceOperationLog - ).where(persistence_operation_log.WorkspaceOperationLog.workspace_uuid == workspace_uuid) + query = ( + sqlalchemy.select(sqlalchemy.func.count()) + .select_from(persistence_operation_log.WorkspaceOperationLog) + .where(persistence_operation_log.WorkspaceOperationLog.workspace_uuid == workspace_uuid) + ) if since is not None: query = query.where(persistence_operation_log.WorkspaceOperationLog.created_at >= since) if level is not None: @@ -1451,10 +1453,7 @@ class WorkspaceSettingsService: started = time.monotonic() try: rows_result = await self.ap.persistence_mgr.execute_async( - sqlalchemy.select(model) - .where(*filters) - .order_by(model.id.desc()) - .limit(MAX_INTEGRITY_SCAN_ROWS) + sqlalchemy.select(model).where(*filters).order_by(model.id.desc()).limit(MAX_INTEGRITY_SCAN_ROWS) ) rows = list(rows_result.all()) @@ -1691,15 +1690,23 @@ class WorkspaceSettingsService: try: model = persistence_operation_log.WorkspaceOperationLog actions = ( - await self.ap.persistence_mgr.execute_async( - sqlalchemy.select(model.action).where(model.workspace_uuid == workspace_uuid).distinct() + ( + await self.ap.persistence_mgr.execute_async( + sqlalchemy.select(model.action).where(model.workspace_uuid == workspace_uuid).distinct() + ) ) - ).scalars().all() + .scalars() + .all() + ) resources = ( - await self.ap.persistence_mgr.execute_async( - sqlalchemy.select(model.resource_type).where(model.workspace_uuid == workspace_uuid).distinct() + ( + await self.ap.persistence_mgr.execute_async( + sqlalchemy.select(model.resource_type).where(model.workspace_uuid == workspace_uuid).distinct() + ) ) - ).scalars().all() + .scalars() + .all() + ) actors = ( await self.ap.persistence_mgr.execute_async( sqlalchemy.select(model.actor_account_uuid, model.actor_name) @@ -1752,18 +1759,20 @@ class WorkspaceSettingsService: try: model = persistence_operation_log.WorkspaceOperationLog oldest_ids = ( - await self.ap.persistence_mgr.execute_async( - sqlalchemy.select(model.id) - .where(model.workspace_uuid == workspace_uuid) - .order_by(model.id.asc()) - .limit(int(count)) + ( + await self.ap.persistence_mgr.execute_async( + sqlalchemy.select(model.id) + .where(model.workspace_uuid == workspace_uuid) + .order_by(model.id.asc()) + .limit(int(count)) + ) ) - ).scalars().all() + .scalars() + .all() + ) if not oldest_ids: return 0 - await self.ap.persistence_mgr.execute_async( - sqlalchemy.delete(model).where(model.id.in_(list(oldest_ids))) - ) + await self.ap.persistence_mgr.execute_async(sqlalchemy.delete(model).where(model.id.in_(list(oldest_ids)))) return len(oldest_ids) except Exception as exc: # pragma: no cover - defensive self.ap.logger.debug(f'Operation log trim skipped: {exc}') @@ -1783,7 +1792,9 @@ class WorkspaceSettingsService: by a privileged click. """ - days = await self.get_retention_days(workspace_uuid) if retention_days is None else clamp_retention(retention_days) + days = ( + await self.get_retention_days(workspace_uuid) if retention_days is None else clamp_retention(retention_days) + ) budget = await self.get_max_rows(workspace_uuid) if max_rows is None else clamp_max_rows(max_rows) cutoff = _utcnow() - datetime.timedelta(days=days) @@ -1816,12 +1827,16 @@ class WorkspaceSettingsService: removed = 0 try: workspaces = ( - await self.ap.persistence_mgr.execute_async( - sqlalchemy.select(persistence_metadata.WorkspaceMetadata.workspace_uuid).where( - persistence_metadata.WorkspaceMetadata.key == OPERATION_LEVEL_KEY + ( + await self.ap.persistence_mgr.execute_async( + sqlalchemy.select(persistence_metadata.WorkspaceMetadata.workspace_uuid).where( + persistence_metadata.WorkspaceMetadata.key == OPERATION_LEVEL_KEY + ) ) ) - ).scalars().all() + .scalars() + .all() + ) except Exception as exc: # pragma: no cover - defensive self.ap.logger.debug(f'Operation log workspace enumeration skipped: {exc}') return {'workspaces': 0, 'removed': 0} diff --git a/src/langbot/pkg/core/app.py b/src/langbot/pkg/core/app.py index 2d3b50b3d..77c0c2839 100644 --- a/src/langbot/pkg/core/app.py +++ b/src/langbot/pkg/core/app.py @@ -440,9 +440,7 @@ class Application: # Operation-log retention piggybacks on the shared maintenance loop # instead of starting another long-lived task, so enabling # traceability adds no scheduling cost of its own. - operation_log_cleanup_cfg = ( - self.instance_config.data.get('operation_log', {}).get('auto_cleanup', {}) - ) + operation_log_cleanup_cfg = self.instance_config.data.get('operation_log', {}).get('auto_cleanup', {}) operation_log_enabled = ( operation_log_cleanup_cfg.get('enabled', True) and self.workspace_settings_service is not None ) diff --git a/src/langbot/pkg/entity/persistence/operation_log.py b/src/langbot/pkg/entity/persistence/operation_log.py index b98ffd67e..9ed005a45 100644 --- a/src/langbot/pkg/entity/persistence/operation_log.py +++ b/src/langbot/pkg/entity/persistence/operation_log.py @@ -17,8 +17,6 @@ Design constraints from __future__ import annotations -import datetime - import sqlalchemy from .base import Base @@ -48,7 +46,9 @@ class WorkspaceOperationLog(Base): __tablename__ = 'workspace_operation_logs' - id = sqlalchemy.Column(sqlalchemy.BigInteger().with_variant(sqlalchemy.Integer, 'sqlite'), primary_key=True, autoincrement=True) + id = sqlalchemy.Column( + sqlalchemy.BigInteger().with_variant(sqlalchemy.Integer, 'sqlite'), primary_key=True, autoincrement=True + ) workspace_uuid = sqlalchemy.Column( sqlalchemy.String(36),