Skip to content

feat(logs): explicit add-a-column form in the column configurator - #69879

Merged
jonmcwest merged 8 commits into
masterfrom
posthog-code/logs-configurator-add-column
Jul 13, 2026
Merged

feat(logs): explicit add-a-column form in the column configurator#69879
jonmcwest merged 8 commits into
masterfrom
posthog-code/logs-configurator-add-column

Conversation

@jonmcwest

Copy link
Copy Markdown
Contributor

Problem

Follow-up to #69275/#69617. The configurator modal's only way to add a column is the "Available columns" taxonomic picker — which exposes only four of the six built-in column types (Timestamp and Source aren't in the Logs taxonomic group at all), and makes "I just want a Level column" a search exercise. There was no way to see the column type vocabulary and pick from it.

Changes

An "Add a column" section between the visible-columns list and the picker:

  • Column type dropdown listing every built-in (Timestamp, Level, Source, Trace ID, Span ID, Message) plus Custom. Options derive from the column registry, so a future built-in type appears without touching the configurator.
  • Name input — optional override, placeholder shows the default label for the chosen type.
  • Expression input, revealed when Custom is selected (monospace, enter-to-add); adding a Custom column without an expression is blocked with a reason, which doubles as the double-submit guard.
  • The form appends to the modal's draft like every other edit — nothing commits or re-runs the query until Save.
  • The taxonomic picker stays below for what it's good at: discovering log/resource attribute keys and one-click adding them as shorthand custom columns.

How did you test this code?

Automated: full logs frontend sweep, 447 tests across 24 suites, all passing. One new test covering the form path end-to-end: invalid custom (no expression) is blocked and adds nothing; a built-in with a name override lands in the draft and the form resets; a custom with an expression round-trips. The reset-on-add ordering matters — kea reducers run before listeners, so resetting the form on submit would wipe the values before the listener reads them; the test locks in the working order. I did not drive the modal in a browser; the branch is on Jon's preview stack.

Docs update

None — behind the same logs-column-configuration flag.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Jon flagged that built-in column types weren't selectable anywhere in the configurator and provided reference designs for an add-a-column flow (type dropdown → name → expression-when-custom). I (Claude, via PostHog Code) implemented it as an inline section of the existing modal rather than a separate dialog, keeping the draft-commit semantics; form state lives in logsColumnConfiguratorLogic per the kea conventions. One bug avoided during implementation: resetting the form in the same-action reducer would race the submit listener (reducers first), so the reset rides on addDraftColumn instead.


Created with PostHog Code

The configurator's only add-path was the available-columns picker, which
exposes just four of the six built-in column types (no Timestamp, no
Source) and buries type selection behind attribute search. Add an
"Add a column" section between the visible list and the picker: a column
type dropdown covering every built-in plus Custom, an optional name
override, and an expression input (shown for Custom, required to add).
Type options derive from the column registry so new built-ins appear
automatically. The taxonomic picker stays for attribute discovery.

Generated-By: PostHog Code
Task-Id: f8dab4c5-0df5-4e26-a33c-44065961a146
@jonmcwest jonmcwest self-assigned this Jul 10, 2026
@jonmcwest
jonmcwest marked this pull request as ready for review July 10, 2026 07:48
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit ecd8e15 · box box-3389b1f13eb7 · ready in 577s (push → usable) · build log · rebuilds on every push, torn down on close

@jonmcwest jonmcwest added the stamphog Request AI approval (no full review) label Jul 10, 2026
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 10, 2026 07:49
@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "feat(logs): explicit add-a-column form i..." | Re-trigger Greptile

@stamphog

stamphog Bot commented Jul 10, 2026

Copy link
Copy Markdown

Note

🤖 stamphog reviewed 5bdab92d1027553f7a2c59d1bad4f6bd29efb073 — verdict: REFUSED

Verified in the diff: the taxonomic picker's onChange also dispatches addDraftColumn, and the new form's state resets on that same action, so a user filling in the new "Add a column" form loses their in-progress input if they instead click a picker entry — the greptile inline comment on this is correct and remains unaddressed on the current head.

  • Author wrote 100% of the modified lines and has 12 merged PRs in these paths (familiarity STRONG).
  • greptile-apps[bot] reviewed the current head.
  • Confirmed: LogsColumnConfigurator.tsx line 210 dispatches addDraftColumn from the taxonomic picker, which the new 'newColumn' reducer resets on — clearing an in-progress Add-a-column form when the user instead picks from the taxonomic list below, as flagged in @greptile-apps's unresolved inline comment on logsColumnConfiguratorLogic.ts
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 124L, 2F substantive, 145L/3F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1c-medium (145L, 3F, single-area, feat)
stamphog 2.0.0b3 .stamphog/policy.yml @ edeede2 · reviewed head 5bdab92

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Jul 10, 2026
@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

The issue is confirmed. The newColumn reducer resets on addDraftColumn (line 73 of the logic diff), but the taxonomic picker's onChange also dispatches addDraftColumn. So any in-progress form state gets wiped when the user picks from the taxonomic list.

The fix is to introduce a dedicated resetNewColumn action and dispatch it from the submitNewColumn listener after addDraftColumn, so the reducer no longer needs to hook into addDraftColumn at all:

logsColumnConfiguratorLogic.ts — add a new action and wire it up:

        // The "Add a column" form (type + name + expression for custom)
        setNewColumnType: (columnType: LogsColumnType) => ({ columnType }),
        setNewColumnName: (name: string) => ({ name }),
        setNewColumnExpression: (expression: string) => ({ expression }),
        resetNewColumn: true,
        submitNewColumn: true,

Then in the newColumn reducer, replace addDraftColumn with resetNewColumn:

                setNewColumnType: (state, { columnType }) => ({ ...state, type: columnType }),
                setNewColumnName: (state, { name }) => ({ ...state, name }),
                setNewColumnExpression: (state, { expression }) => ({ ...state, expression }),
                // Reset on a dedicated action so the taxonomic picker's addDraftColumn
                // doesn't wipe an in-progress form
                resetNewColumn: () => ({ type: 'custom' as const, name: '', expression: '' }),
                openConfigurator: () => ({ type: 'custom' as const, name: '', expression: '' }),

And in the submitNewColumn listener, dispatch resetNewColumn after addDraftColumn:

        submitNewColumn: () => {
            const { type, name, expression } = values.newColumn
            if (values.newColumnError) {
                return
            }
            actions.addDraftColumn({
                type,
                ...(name.trim() ? { name: name.trim() } : {}),
                ...(type === 'custom' ? { expression: expression.trim() } : {}),
            })
            actions.resetNewColumn()
        },

This keeps the reset-after-read ordering (the listener still reads values.newColumn before dispatching either action) while decoupling the form reset from the shared addDraftColumn action. The test assertion expect(logic.values.newColumn).toEqual({ type: 'custom', name: '', expression: '' }) after submitNewColumn will still hold — resetNewColumn runs as part of the same listener tick.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

Bundle size — no change

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 70.09 MiB · no change

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.21 MiB · 22 files no change ███░░░░░░░ 28.1% of 4.29 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.09 MiB · 2,971 files 🔺 +110 B (+0.0%) █████████░ 87.5% of 9.25 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
789 B src/scenes/ChunkLoadErrorBoundary.tsx
668 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
277.3 KiB ../node_modules/.pnpm/posthog-js@1.399.1/node_modules/posthog-js/dist/rrweb.js
266.9 KiB ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
224.6 KiB src/taxonomy/core-filter-definitions-by-group.json
221.5 KiB ../node_modules/.pnpm/posthog-js@1.399.1/node_modules/posthog-js/dist/module.js
164.0 KiB src/queries/validators.js
154.0 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
105.9 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

⚠️ Dist folder size — 🔺 +21.0 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1438.52 MiB · 🔺 +21.0 KiB (+0.0%)

@trunk-io

trunk-io Bot commented Jul 10, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

The newColumn form reducer reset on addDraftColumn, but the available-columns
picker dispatches that same action. Picking a column there wiped an in-progress
add-column form. Reset only via a dedicated resetNewColumn action fired by the
submit listener after it reads the form values.

Generated-By: PostHog Code
Task-Id: 811d9a61-3f2c-4538-ab25-3d392a6bea5b
@github-actions
github-actions Bot requested a deployment to preview-pr-69879 July 10, 2026 09:32 In progress
Generated-By: PostHog Code
Task-Id: 811d9a61-3f2c-4538-ab25-3d392a6bea5b

Copy link
Copy Markdown
Contributor Author

Iteration status:

  • Review feedback: The one open thread (form-clearing on available-column pick) is fixed in d3d0c3b and resolved, with a regression test added.
  • Required CI checks: All green (frontend typecheck, Jest EE/FOSS, Playwright, Python, Dagster, LLM services, shellcheck). The earlier failures were pre-existing master issues (textCard markdown test + unrelated Customer Analytics snapshot) and cleared once master was merged in.
  • Mergeable: No conflicts; master merged in cleanly and the diff remains scoped to the three logs configurator files.

Two remaining gates need a human and can't be resolved from here:

  1. Review required from @PostHog/logs — needs an approving review.
  2. Visual Review (Complete Visual Review run / Visual regression tests pass) is red because the external service flags 29 changed snapshots for design approval. These are not from this PR's diff (it touches only three logs .ts files, adds no stories or committed snapshots, and all 16 committed-snapshot chromium shards pass) — they look like baseline drift picked up via the master merge, and need a maintainer to review/approve in the visual review UI.

Generated-By: PostHog Code
Task-Id: 3dab2bb7-b1e1-43fe-9915-5430f04dd670
Extract the custom-expression validation into one customColumnExpressionError
helper shared by the add-form and draft error selectors, drop the redundant
COLUMN_TYPE_OPTIONS casts, adopt LemonField.Pure for the form labels to match
the sibling logs forms, and share the expression-input placeholder constant.

Generated-By: PostHog Code
Task-Id: f8dab4c5-0df5-4e26-a33c-44065961a146
Generated-By: PostHog Code
Task-Id: 3dab2bb7-b1e1-43fe-9915-5430f04dd670

Copy link
Copy Markdown
Contributor Author

Iteration status (updated):

  • Review feedback: the one review thread (form-clearing when picking from the available-columns list) is fixed and resolved, with a regression test. No unresolved threads remain.
  • CI: green on the latest commit. All required checks pass.
    • Visual review: now reports "No visual changes" on the up-to-date branch. The earlier flag of ~29 changed snapshots was baseline drift and cleared once master was merged in; none of the diffs came from this PR (it touches only the logs column-configurator files and adds no stories or committed snapshots).
    • A Customer Analytics AccountsTab story shard failed once with a waitForSelector timeout unrelated to this PR; it passed on re-run (confirmed flaky) and passed again cleanly on the latest run.
  • Mergeable: no conflicts; master merged in server-side and the diff stays scoped to the four logs configurator files.

Remaining gate: this needs an approving review from @PostHog/logs — a required human review that can't be satisfied automatically. Everything else is green.

@jonmcwest jonmcwest added the stamphog Request AI approval (no full review) label Jul 10, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Contained frontend feature (new add-column form in a logs product configurator, behind a feature flag), no risky territory touched; the one Greptile review concern was fixed with a regression test and resolved, author has strong familiarity and owns the files.

  • Author wrote 100% of the modified lines and has 12 merged PRs in these paths (familiarity STRONG).
  • 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 140L, 3F substantive, 172L/4F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1c-medium (172L, 4F, single-area, feat)
stamphog 2.0.0b3 .stamphog/policy.yml @ f6aa24a · reviewed head 4a4e31b

@posthog

posthog Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

👋 Visual changes detected for this PR.

Review and approve in PostHog Visual Review

If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix.

Generated-By: PostHog Code
Task-Id: 9e17d727-cfb7-4839-8c23-61b226f87591
@jonmcwest
jonmcwest merged commit c9df32f into master Jul 13, 2026
191 checks passed
@jonmcwest
jonmcwest deleted the posthog-code/logs-configurator-add-column branch July 13, 2026 08:22
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-13 08:53 UTC Run
prod-us ✅ Deployed 2026-07-13 09:08 UTC Run
prod-eu ✅ Deployed 2026-07-13 09:08 UTC Run

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

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant