feat: [ENG-1897] Create brv query-log view oclif command

[bao-byterover]

Summary

• Problem: No CLI way to inspect query log entries outside the daemon/REPL -- developers and support must dig through raw files to debug query resolution behavior.
• Why it matters: brv query-log view gives instant, filterable access to query history without starting the daemon, unblocking the Query Recall Metrics & Value Visibility project (ms1).
• What changed: Added brv query-log view oclif command with flags (--since, --before, --status, --tier, --limit, --format, --detail) and optional positional id arg. Extracted parseTimeFilter from curate/view.ts into a shared utility (src/oclif/lib/time-filter.ts). Created stub domain types, interface, store, and use-case for downstream tickets (ENG-1887/1888/1889/1896) to implement.
• What did NOT change (scope boundary): No actual query log reading/formatting logic -- FileQueryLogStore and QueryLogUseCase are stubs. No daemon changes. No TUI changes.

Type of change

• Bug fix
• New feature
• Refactor (no behavior change)
• Documentation
• Test
• Chore (build, dependencies, CI)

Scope (select all touched areas)

• TUI / REPL
• Agent / Tools
• LLM Providers
• Server / Daemon
• Shared (constants, types, transport events)
• CLI Commands (oclif)
• Hub / Connectors
• Cloud Sync
• CI/CD / Infra

Linked issues

• Closes ENG-1897
• Related ENG-1887, ENG-1888, ENG-1889, ENG-1896

Root cause (bug fixes only, otherwise write N/A)

N/A

Test plan

• Coverage added:
  • Unit test
  • Integration test
  • Manual verification only
• Test file(s):
  • test/commands/query-log/view.test.ts -- command integration tests (flag parsing, store instantiation, use-case delegation, error handling)
  • test/unit/oclif/lib/time-filter.test.ts -- unit tests for parseTimeFilter (relative: m/h/d/w, absolute: ISO date/datetime, invalid input)
• Key scenario(s) covered:
  • Default flags forwarded correctly (limit=10, format=text, detail=false)
  • Each flag (--limit, --format, --detail, --status, --tier, --since, --before) parsed and delegated individually
  • Multiple --status and --tier flags combined
  • Positional id arg forwarded
  • All flags + arg combined in a single invocation
  • Invalid --since / --before values exit with code 2
  • createDependencies instantiates real QueryLogUseCase from FileQueryLogStore
  • parseTimeFilter: relative (30m, 1h, 24h, 7d, 2w, 0m, 999d), absolute (date, datetime), invalid (empty, partial, unsupported unit)

User-visible changes

Change	Detail
New command	brv query-log view available (currently stubs -- outputs nothing until ENG-1896 implements use-case)
Shared utility	parseTimeFilter extracted from curate/view.ts -- no behavior change for existing brv curate view

Evidence

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording

(Tests to be run by reviewer -- attach results here)

Checklist

• Tests added or updated and passing (npm test)
• Lint passes (npm run lint)
• Type check passes (npm run typecheck)
• Build succeeds (npm run build)
• Commits follow Conventional Commits format
• Documentation updated (if applicable)
• No breaking changes (or clearly documented above)
• Branch is up to date with main

Risks and mitigations

Risk	Mitigation
Stub store/use-case could drift from downstream implementation (ENG-1888/1896)	Stubs implement the same IQueryLogUseCase interface that downstream tickets must fulfill -- type-checking catches drift
parseTimeFilter extraction could break curate/view	Existing curate tests validate behavior is preserved; function signature unchanged

[github-actions]

Claude finished @bao-byterover's task in 2m 33s —— View job

---

PR Review: feat: [ENG-1897] Create brv query-log view oclif command

• Gather context and read changed files
• Review implementation files
• Review test files
• Analyze architecture compliance (Outside-In principle)
• Post final review

---

Overall Score: 4/5

Good PR. The outside-in scaffolding is intentional and well-executed — stubs are clearly marked with follow-up ticket references (ENG-1887/1888/1889/1895/1896), the createDependencies pattern is a testability improvement over curate/view.ts, and the as const runtime-derived type pattern for STATUSES/TIERS is cleaner than the local VALID_STATUSES array in the curate counterpart. Tests are thorough and well-structured. A few issues worth addressing:

---

Issues

suggestion src/oclif/commands/query-log/view.ts:33,50 — Minutes (m) missing from flag descriptions

The since and before flag descriptions advertise 1h, 24h, 7d, 2w but the implementation also supports minutes (e.g., 30m). The error message at line 101 has the same gap.

// Current:
description: 'Show entries started before this time (ISO date or relative: 1h, 24h, 7d, 2w)',

// Should be:
description: 'Show entries started before this time (ISO date or relative: 30m, 1h, 24h, 7d, 2w)',

Same fix needed for the --since flag and the parseTime error message. Fix this →

---

issue src/oclif/commands/query-log/view.ts:4 — Command imports domain constants through storage interface

The command imports QUERY_LOG_STATUSES, QUERY_LOG_TIERS, QueryLogStatus, QueryLogTier from i-query-log-store.js. The storage interface is a re-export stub whose sole purpose right now is forwarding from the entity. This creates a dependency on a storage layer file just to get domain constants. The entity (query-log-entry.ts) is the declared single source of truth — importing directly from it would be cleaner and avoids coupling the command to the storage layer's namespace.

// Current:
import {QUERY_LOG_STATUSES, QUERY_LOG_TIERS, type QueryLogStatus, type QueryLogTier}
  from '../../../server/core/interfaces/storage/i-query-log-store.js'

// Cleaner:
import {QUERY_LOG_STATUSES, QUERY_LOG_TIERS, type QueryLogStatus, type QueryLogTier}
  from '../../../server/core/domain/entities/query-log-entry.js'

---

nitpick src/server/infra/usecase/query-log-use-case.ts:9 — queryLogStore: unknown needs a follow-up guard

type QueryLogUseCaseDeps = {
  queryLogStore: unknown   // ← intentional stub, but easy to forget
  terminal: Terminal
}

The stub is understandable and clearly commented, but once IQueryLogStore lands in ENG-1888, this must be updated before the real use case (ENG-1896) ships. Consider adding a TODO(ENG-1888) inline comment so the search is trivial:

queryLogStore: unknown // TODO(ENG-1888): replace with IQueryLogStore once interface is defined

---

nitpick src/server/core/domain/entities/query-log-entry.ts — File contains no entity

The file only exports constants and derived types — no QueryLogEntry type exists yet. The stub comment is clear, but the filename (query-log-entry.ts) implies an entity is defined here. No action needed now (ENG-1887 will fix this), but consider whether a query-log-constants.ts name would be more honest for the stub phase.

---

Praise

praise src/oclif/commands/query-log/view.ts:64–71 — createDependencies pattern is a real improvement

Extracting dependency construction into a protected method makes the command testable without property injection or full oclif runner plumbing. The TestableQueryLogView subclass in the test leverages this cleanly. Worth applying to curate/view.ts in a follow-up.

praise src/server/core/domain/entities/query-log-entry.ts:9–13 — as const → derived type pattern

export const QUERY_LOG_STATUSES = ['cancelled', 'completed', 'error', 'processing'] as const
export type QueryLogStatus = (typeof QUERY_LOG_STATUSES)[number]

This is strictly better than curate/view.ts's local VALID_STATUSES: CurateLogStatus[], which is a widened type that can drift from the union. The runtime array can be used directly in oclif options: and in type guards without duplication.

praise test/commands/query-log/view.test.ts:192–207 — RealDepsQueryLogView instantiation test

Testing that createDependencies actually wires up a QueryLogUseCase instance (not just a mock) catches construction regressions without integration overhead. Clever pattern.

---

[github-actions]

Claude finished @bao-byterover's task in 2m 13s —— View job

---

PR Review: feat: [ENG-1897] Create brv query-log view oclif command

• Gather context and read changed files
• Review implementation files
• Review test files
• Analyze architecture compliance (Outside-In principle)
• Post final review

---

Overall Score: 4.5/5

Good follow-up commit — the two main issues from the prior review are both addressed: flag descriptions now include 30m, and the import in query-log/view.ts has been corrected to come directly from the entity. The extra fix to curate/view.ts descriptions and error message is a nice bonus. A couple of minor items remain open.

---

Fixed from Prior Review ✓

✅ src/oclif/commands/query-log/view.ts:4 — Import now correctly points to query-log-entry.ts directly, not via the storage interface stub.

✅ src/oclif/commands/query-log/view.ts:33,50,100 — All three strings (--before, --since descriptions, error message) now include 30m.

✅ src/oclif/commands/curate/view.ts — Descriptions and error message in the curate command also updated to include 30m — good catch to fix both in the same commit.

---

Remaining Issues

nitpick src/server/infra/usecase/query-log-use-case.ts:9 — queryLogStore: unknown still has no TODO(ENG-1888) marker. As suggested in the prior review, without it this is easy to miss during ENG-1896 implementation. Fix this →

nitpick src/server/core/interfaces/usecase/i-query-log-use-case.ts:4 — The command's import was fixed to pull QueryLogStatus/QueryLogTier directly from the entity, but the use-case interface still imports them through i-query-log-store.ts. Minor inconsistency — they both resolve to the same source, but the interface is creating a dependency on the storage layer when neither file has any storage concern yet. Fix this →

---

Observations (no action needed)

src/oclif/commands/query-log/view.ts:91–92 — The status and tier runtime filters (filter((s) => STATUSES.includes(s))) are technically redundant since oclif already validates against the declared options: array before run() is called. This is the same pattern in curate/view.ts and is harmless — just slightly defensive code that doesn't add runtime value.

src/oclif/commands/query-log/view.ts:60 — QUERY_LOG_TIERS.map(String) for oclif options (strings) + map(Number) back in run() is the correct approach for numeric options in oclif, and the symmetry is clear enough. No issue.

src/server/core/domain/entities/query-log-entry.ts — File contains no entity yet (ENG-1887 scope). The filename is aspirational for now, which is fine given the clear stub comment referencing the follow-up ticket.

---

Praise

src/oclif/commands/query-log/view.ts:64–71 — createDependencies protected pattern is clean and the test leverages it well via TestableQueryLogView. This is a genuine improvement over the direct instantiation in curate/view.ts.

test/commands/query-log/view.test.ts:192–207 — RealDepsQueryLogView.exposedCreateDependencies test catches wire-up regressions without requiring a full oclif runner. Solid pattern.

test/unit/oclif/lib/time-filter.test.ts — Edge cases well covered: 0m, 999d, partial formats, unsupported units, empty string. No gaps.

---