Skip to content

feat(viewer): add redacted diagnostics event API - #1648

Open
kunickiaj wants to merge 1 commit into
mainfrom
adam/feat-viewer-redacted-diagnostics-api
Open

feat(viewer): add redacted diagnostics event API#1648
kunickiaj wants to merge 1 commit into
mainfrom
adam/feat-viewer-redacted-diagnostics-api

Conversation

@kunickiaj

@kunickiaj kunickiaj commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Description

Adds the redacted diagnostics event contract and GET /api/diagnostics/events, including bounded source queries, authenticated cursor pagination, stable ordering, safe recovery metadata, and adversarial redaction coverage. The design document records the privacy and interaction contract used by the rest of the stack.

Type of Change

  • 🚀 Feature (new functionality)
  • 🧪 Testing (test-only changes)

Testing

  • Relevant checks pass locally (pnpm run tsc, pnpm run lint, pnpm run test)
  • Added/updated tests for changes
  • Manually verified changes work as expected

Checklist

  • Code follows project style (pnpm run lint passes for touched files)
  • Self-review completed
  • Documentation updated (if needed)
  • No new warnings introduced

Copy link
Copy Markdown
Owner Author

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T05:39:50.621152Z 270265e New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ecde519870

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/viewer-server/src/routes/diagnostics.ts
Comment thread packages/viewer-server/src/routes/diagnostics.ts
Comment thread packages/viewer-server/src/routes/diagnostics.ts
@kunickiaj
kunickiaj force-pushed the adam/feat-viewer-redacted-diagnostics-api branch from ecde519 to 4b6ef61 Compare September 8, 2026 02:49

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b6ef61614

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/viewer-server/src/routes/diagnostics.ts Outdated
@kunickiaj
kunickiaj force-pushed the adam/feat-viewer-redacted-diagnostics-api branch from 4b6ef61 to 03a2437 Compare September 8, 2026 05:17

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 03a24375b2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ELSE 'unspecified'
END AS category
FROM raw_event_flush_batches
WHERE status IN ('error', 'failed', 'gave_up') ${providerFilter}`;

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 Bound observer failure lookup before sorting

On databases with many retained failed or retry-exhausted flush batches, the default diagnostics request evaluates every matching row before the outer ORDER BY ... LIMIT in boundedSourceSelect. I checked this query against the existing (status, updated_at) index: because it filters three statuses and globally orders by updated_at plus the computed key, SQLite reports a temporary B-tree and scans all terminal failures. Since these rows have no retention cleanup and the drawer is intended to poll, select an indexed recent window per status or add an order-compatible access path before cross-source sorting.

Useful? React with 👍 / 👎.

Comment on lines +229 to +233
FROM raw_event_sessions AS sessions
WHERE EXISTS (SELECT 1 FROM raw_events AS events
WHERE events.source = sessions.source
AND events.stream_id = sessions.stream_id
AND events.event_seq > sessions.last_flushed_event_seq)

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 Avoid rescanning every capture session for backlog totals

When a long-running database has accumulated many raw_event_sessions, every default or capture diagnostics request scans the entire sessions table and runs a correlated raw_events existence lookup for each row before sorting and applying LIMIT 1000. Purged sessions remain in this table, and its only index is the (source, stream_id) primary key, so the limit bounds aggregation output but not polling work; derive the backlog from an indexed pending-session set or otherwise bound the session scan first.

Useful? React with 👍 / 👎.

@kunickiaj
kunickiaj force-pushed the adam/feat-viewer-redacted-diagnostics-api branch from 03a2437 to 270265e Compare September 8, 2026 05:34

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 270265e3c1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

} while (cursor);

expect(events).toHaveLength(17);
expect(new Set(events.map((event) => event.id))).toHaveLength(17);

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 Assert the Set's size instead of its length

When this pagination test runs, toHaveLength(17) inspects a length property, but a JavaScript Set exposes its count through size; the assertion therefore fails even when all 17 event IDs are unique, breaking the repository's test gate. Assert new Set(...).size with toBe(17) as the earlier pagination assertion does.

AGENTS.md reference: AGENTS.md:L18-L19

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant