Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions gui/src/components/storage-workspace/StorageWorkspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ export interface CodexLogGuardReport {
reclaimableBytes: number;
estimatedLogBytes: number | null;
};
metricsSkipped?: null | {
reason: "database_too_large";
thresholdBytes: number;
};
}

export interface StorageReport {
Expand Down Expand Up @@ -205,6 +209,22 @@ function CodexLogGuardPanel({
</div>
</>
)}
{/*
Say WHY the rows are missing (#2605). Skipping the aggregates above the size threshold
is what keeps a cold inspection off the proxy thread, but silently dropping the row
block reads as "this database has no rows" — the exact confusion the null-vs-zero
distinction on the server exists to prevent. A user who sees a 1 GB database and no
row count deserves the reason.
*/}
{!metrics && report.metricsSkipped && (
<div className="stw-kv-row" data-testid="log-guard-metrics-skipped">
<dt>{t("storage.col.rows")}</dt>
<dd className="muted">
{logGuardLabel(locale, "metricsSkippedLarge")
.replace("{threshold}", formatBytes(report.metricsSkipped.thresholdBytes, locale))}
</dd>
</div>
)}
<div className="stw-kv-row">
<dt><code>sqlite_home</code></dt>
<dd className="stw-kv-mono">
Expand Down
10 changes: 10 additions & 0 deletions gui/src/i18n/log-guard-labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export type LogGuardLabelKey =
| "inspectionOnly"
| "externalSqliteHome"
| "inspectionUnavailable"
| "metricsSkippedLarge"
| "protection"
| "compat"
| "quiet"
Expand Down Expand Up @@ -37,6 +38,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: 'Inspection only',
externalSqliteHome: 'External SQLite storage',
inspectionUnavailable: "Diagnostic log inspection is unavailable.",
metricsSkippedLarge: "Row metrics skipped: the database is above {threshold}, and scanning it would stall the proxy.",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Move skipped-metrics copy into the canonical locale catalogs

This new visible string is added to the bespoke log-guard-labels.ts map rather than to src/i18n/en.ts and every locale module, and the component renders it through logGuardLabel instead of t(). That bypasses the repository's canonical TKey typing and automatic locale discovery, allowing this copy to drift outside the normal i18n validation path; add a standard locale key and render it through t("key").

AGENTS.md reference: gui/AGENTS.md:L14-L18

Useful? React with 👍 / 👎.

protection: "Protection",
compat: "Compatibility",
quiet: "Quiet",
Expand All @@ -63,6 +65,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: 'Nur Inspektion',
externalSqliteHome: 'Externer SQLite-Speicher',
inspectionUnavailable: "Die Diagnoseprotokoll-Inspektion ist nicht verfügbar.",
metricsSkippedLarge: "Zeilenmetriken übersprungen: Die Datenbank ist größer als {threshold}; ein Scan würde den Proxy blockieren.",
protection: "Schutz",
compat: "Kompatibilität",
quiet: "Leise",
Expand All @@ -89,6 +92,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: "Inspection uniquement",
externalSqliteHome: "Stockage SQLite externe",
inspectionUnavailable: "L’inspection des journaux de diagnostic est indisponible.",
metricsSkippedLarge: "Métriques de lignes ignorées : la base dépasse {threshold} et son analyse bloquerait le proxy.",
protection: "Protection",
compat: "Compatibilité",
quiet: "Silencieux",
Expand All @@ -115,6 +119,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: '검사 전용',
externalSqliteHome: '외부 SQLite 저장소',
inspectionUnavailable: "진단 로그 검사를 사용할 수 없습니다.",
metricsSkippedLarge: "행 지표를 건너뛰었습니다. 데이터베이스가 {threshold}보다 커서 스캔하면 프록시가 멈춥니다.",
protection: "보호",
compat: "호환 모드",
quiet: "조용한 모드",
Expand All @@ -141,6 +146,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: '仅检查',
externalSqliteHome: '外部 SQLite 存储',
inspectionUnavailable: "诊断日志检查当前不可用。",
metricsSkippedLarge: "已跳过行指标:数据库超过 {threshold},扫描会阻塞代理。",
protection: "保护",
compat: "兼容模式",
quiet: "静默模式",
Expand All @@ -167,6 +173,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: '僅檢查',
externalSqliteHome: '外部 SQLite 儲存空間',
inspectionUnavailable: "診斷記錄檢查目前無法使用。",
metricsSkippedLarge: "已略過列指標:資料庫超過 {threshold},掃描會阻塞代理。",
protection: "保護",
compat: "相容模式",
quiet: "靜默模式",
Expand All @@ -193,6 +200,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: 'Только проверка',
externalSqliteHome: 'Внешнее хранилище SQLite',
inspectionUnavailable: "Проверка диагностических журналов недоступна.",
metricsSkippedLarge: "Метрики строк пропущены: база больше {threshold}, и её сканирование заблокировало бы прокси.",
protection: "Защита",
compat: "Совместимость",
quiet: "Тихий режим",
Expand All @@ -219,6 +227,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: '検査のみ',
externalSqliteHome: '外部 SQLite ストレージ',
inspectionUnavailable: "診断ログの検査を利用できません。",
metricsSkippedLarge: "行メトリクスをスキップしました。データベースが {threshold} を超えており、走査するとプロキシが停止します。",
protection: "保護",
compat: "互換モード",
quiet: "静音モード",
Expand All @@ -245,6 +254,7 @@ const LABELS: Record<Locale, Record<LogGuardLabelKey, string>> = {
inspectionOnly: 'Yalnızca inceleme',
externalSqliteHome: 'Harici SQLite depolaması',
inspectionUnavailable: "Tanılama günlüğü incelemesi kullanılamıyor.",
metricsSkippedLarge: "Satır ölçümleri atlandı: veritabanı {threshold} sınırının üzerinde ve taranması proxy’yi kilitler.",
protection: "Koruma",
compat: "Uyumluluk",
quiet: "Sessiz",
Expand Down
40 changes: 40 additions & 0 deletions gui/tests/storage-log-guard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,43 @@ test("Storage overview does not render arbitrary Log Guard error strings", () =>
expect(html).not.toContain("/private/state/logs_2.sqlite");
expect(html).not.toContain("failed");
});

/**
* A skipped scan must SAY it was skipped (#2605).
*
* Above the size threshold the server returns `metrics: null` so a cold inspection cannot stall
* the proxy thread. Rendering that as an absent row block reads as "this database has no rows" —
* the exact confusion the server's null-vs-zero distinction exists to prevent, and the more
* misleading the larger the database actually is.
*/
test("a skipped large-database scan states the reason instead of rendering no rows", () => {
const large = report();
large.codexLogs!.files.databaseBytes = 1_468_923_904;
large.codexLogs!.metrics = null;
large.codexLogs!.metricsSkipped = { reason: "database_too_large", thresholdBytes: 67_108_864 };

const html = renderToStaticMarkup(
<LanguageProvider>
<StorageWorkspace report={large} locale="en" />
</LanguageProvider>,
);

expect(html).toContain('data-testid="log-guard-metrics-skipped"');
expect(html).toContain("Row metrics skipped");
// The threshold is stated, so the reader can tell why this database crossed it.
expect(html).toContain("64 MiB");
// The file sizes still render: only the row aggregates were skipped, not the inspection.
expect(html).toContain("1.4 GiB");
// And it must not silently show a row count it never computed.
expect(html).not.toContain(">400<");
});

test("a database under the threshold still renders full row metrics", () => {
const html = renderToStaticMarkup(
<LanguageProvider>
<StorageWorkspace report={report()} locale="en" />
</LanguageProvider>,
);
expect(html).not.toContain('data-testid="log-guard-metrics-skipped"');
expect(html).toContain("400");
});
26 changes: 22 additions & 4 deletions src/codex/log-guard/inspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import {

const IMMUTABLE_READONLY_FLAGS = constants.SQLITE_OPEN_READONLY | constants.SQLITE_OPEN_URI;
const KNOWN_LOG_LEVELS = new Set(["TRACE", "DEBUG", "INFO", "WARN", "ERROR"]);
// The issue reporter measured a ~1 GB database taking 17.3s for GROUP BY level
// alone; skipping all row aggregates above 64 MiB reduced /api/storage to 628ms.
const MAX_SYNCHRONOUS_METRICS_DATABASE_BYTES = 64 * 1024 * 1024;

interface CurrentLogColumn {
name: string;
Expand Down Expand Up @@ -115,6 +118,10 @@ export interface CodexLogGuardInspection {
reclaim: CodexLogGuardCapability;
};
metrics: CodexLogGuardMetrics | null;
metricsSkipped: null | {
reason: "database_too_large";
thresholdBytes: number;
};
}

interface ColumnRow {
Expand Down Expand Up @@ -167,9 +174,8 @@ function fileSize(path: string): number {
* repeated stalls without ever serving stale numbers: any write changes the WAL
* and invalidates the entry.
*
* This bounds the repeat cost, not the first one. A cold inspection of a huge
* database still blocks; moving that work off-thread needs a Worker and is
* tracked separately.
* Memoization bounds repeat cost. The database-size gate below separately bounds
* cold request-thread work by omitting these aggregates for large databases.
*/
type InspectionCacheEntry = {
key: string;
Expand Down Expand Up @@ -233,6 +239,7 @@ function unavailableInspection(): CodexLogGuardInspection {
reclaim: unavailable,
},
metrics: null,
metricsSkipped: null,
};
}

Expand Down Expand Up @@ -425,6 +432,7 @@ function inspectCodexLogsUncached(deps: CodexSqliteHomeDeps = {}): CodexLogGuard
...common,
schema,
metrics: null,
metricsSkipped: null,
capabilities: {
inspection: { state: "supported" },
protection: mutation,
Expand All @@ -440,6 +448,7 @@ function inspectCodexLogsUncached(deps: CodexSqliteHomeDeps = {}): CodexLogGuard
...common,
schema,
metrics: null,
metricsSkipped: null,
capabilities: {
inspection: { state: "supported" },
protection: mutation,
Expand All @@ -458,6 +467,7 @@ function inspectCodexLogsUncached(deps: CodexSqliteHomeDeps = {}): CodexLogGuard
...common,
schema,
metrics: null,
metricsSkipped: null,
capabilities: {
inspection: { state: "supported" },
protection: mutation,
Expand All @@ -476,10 +486,17 @@ function inspectCodexLogsUncached(deps: CodexSqliteHomeDeps = {}): CodexLogGuard
? { state: "compatible" }
: { state: "unsupported", reason: "unknown_schema" };
const mutation = capabilityFor(schema);
const metricsSkipped = files.databaseBytes > MAX_SYNCHRONOUS_METRICS_DATABASE_BYTES
? {
reason: "database_too_large" as const,
thresholdBytes: MAX_SYNCHRONOUS_METRICS_DATABASE_BYTES,
}
: null;
return {
...common,
schema,
metrics: readMetrics(db, columns),
metrics: metricsSkipped === null ? readMetrics(db, columns) : null,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep reclaim metadata when skipping row scans

For every compatible database above 64 MiB, this sets the entire metrics object to null, even though page_size and freelist_count are cheap PRAGMAs rather than unbounded row aggregates. In the inspected Storage dashboard, reclaimAvailable requires metrics.reclaimableBytes > 0, so the Compact section now disappears for every large database—including one with substantial reclaimable space, which is precisely where compaction is most useful. Preserve the freelist metadata separately while skipping only the expensive row queries, or otherwise keep the action available.

Useful? React with 👍 / 👎.

metricsSkipped,
Comment on lines +498 to +499

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Report skipped metrics in the doctor output

When a database crosses this threshold, formatCodexLogGuardDoctor only prints its metrics line when report.metrics is present and never examines metricsSkipped. Consequently, ocx doctor silently loses the row, TRACE, and reclaimable-space diagnostics without explaining that the scan was deliberately skipped—the same ambiguity this change fixes in the dashboard. Add a doctor-formatting branch for metricsSkipped and focused coverage for the large-database report.

Useful? React with 👍 / 👎.

capabilities: {
inspection: { state: "supported" },
protection: mutation,
Expand All @@ -496,6 +513,7 @@ function inspectCodexLogsUncached(deps: CodexSqliteHomeDeps = {}): CodexLogGuard
...common,
schema,
metrics: null,
metricsSkipped: null,
capabilities: {
inspection: { state: "supported" },
protection: mutation,
Expand Down
21 changes: 21 additions & 0 deletions tests/codex-log-guard-inspect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
renameSync,
rmSync,
statSync,
truncateSync,
utimesSync,
unlinkSync,
writeFileSync,
Expand Down Expand Up @@ -113,6 +114,26 @@ describe("Codex Log Guard inspection", () => {
expect(report.metrics?.traceShare).toBe(0.5);
expect(report.metrics?.topTargets[0]).toEqual({ target: "TARGET_1", rows: 2 });
expect(report.metrics?.reclaimableBytes).toBeGreaterThanOrEqual(0);
expect(report.metricsSkipped).toBeNull();
});

test("skips row aggregates for a large database without reporting zero metrics", () => {
const root = makeRoot();
const databasePath = join(root, "logs_2.sqlite");
createCurrentLogsDb(databasePath);
truncateSync(databasePath, 64 * 1024 * 1024 + 1);

const report = inspectCodexLogs({ codexHome: root });

expect(report.schema).toEqual({ state: "compatible" });
expect(report.metrics).toBeNull();
expect(report.metricsSkipped).toEqual({
reason: "database_too_large",
thresholdBytes: 64 * 1024 * 1024,
});
expect(report).not.toMatchObject({
metrics: { totalRows: 0, rowsByLevel: {}, estimatedLogBytes: 0 },
});
});

test("never exposes feedback bodies, arbitrary levels, target names, or paths", () => {
Expand Down
Loading