Skip to content

Analytics foundation: track_event hook + core instrumentation (COPLAN-20)#116

Merged
HamptonMakes merged 2 commits into
mainfrom
hampton/coplan-20/analytics-foundation
Jun 1, 2026
Merged

Analytics foundation: track_event hook + core instrumentation (COPLAN-20)#116
HamptonMakes merged 2 commits into
mainfrom
hampton/coplan-20/analytics-foundation

Conversation

@HamptonMakes
Copy link
Copy Markdown
Collaborator

Resolves COPLAN-20.

What

Lays down the engine-side event-tracking layer that the rest of Cycle 2's KPIs depend on.

  • New CoPlan.configuration.track_event hook (no-op default). Mirrors the existing notification_handler shape so hosts have one consistent pattern to wire.

  • New CoPlan::Analytics.track(event, user:, **props) facade used at every call site.

  • Instrumentation:

    Event Site
    page_view ApplicationController#track_page_view — fires once per successful 2xx HTML GET, skipping Turbo Frame requests, agent/non-browser requests, and non-HTML responses
    plan_created Plans::Create — emitted after the DB transaction commits
    plan_published PlansController#update_status — only on the brainstorm → considering transition
    comment_created Comment after_create_commit
    thread_resolved CommentThread#resolve!

Handler errors are swallowed and reported via error_reporter so a broken analytics sink never breaks a user request.

Scope decisions

  • search_performed / search_result_clicked deferred. Plans index has no text search yet — only filter-by-status/tag/plan_type and no result-click handler. Emitting events with no real semantics would just produce misleading data. They'll land alongside the actual search feature.
  • Destination recommendation: MySQL analytics_events table in the host (coplan-square). It joins cleanly to coplan_plans / coplan_users for every Cycle 2 KPI, takes ~1 day to wire, and is trivial to dual-write to Snowflake later by extending the handler. This PR only ships the engine hook; the host-side wiring is a follow-up PR in coplan-square that I'll confirm with Hampton first.

Testing

  • 19 new specs covering the facade, every call site, and the page_view skip conditions (Turbo Frame, agent UA, non-2xx).
  • Full suite: bundle exec rspec888 examples, 0 failures.

Host docs

Updated docs/HOST_APP_GUIDE.md Configuration reference with the new track_event parameter.

…-20)

Adds CoPlan.configuration.track_event — a no-op-by-default lambda that
the host wires to a destination (MySQL events table / Snowflake /
Datadog). Mirrors the existing notification_handler shape.

CoPlan::Analytics.track is a thin facade used at call sites:
  - page_view         — ApplicationController#track_page_view (2xx HTML
                        GETs only; skips Turbo Frame and agent traffic)
  - plan_created      — Plans::Create (fires after the DB transaction
                        commits, so rolled-back creates do not emit)
  - plan_published    — PlansController#update_status, only on the
                        brainstorm→considering transition
  - comment_created   — Comment after_create_commit
  - thread_resolved   — CommentThread#resolve!

Handler errors are swallowed and reported via error_reporter so a
broken analytics sink never breaks a user request.

The search_performed / search_result_clicked events from the ticket
are deferred — plans index has no text search yet, and emitting fake
events would just produce misleading data. They'll land with the
search feature.

Specs (19 new examples; full suite 888/888 green):
  - spec/lib/coplan/analytics_spec.rb               — facade contract
  - spec/services/plans/create_spec.rb              — plan_created
  - spec/models/coplan/comment_analytics_spec.rb    — comment_created
  - spec/models/coplan/comment_thread_analytics_spec.rb — thread_resolved
  - spec/requests/analytics_instrumentation_spec.rb — page_view + plan_published
  - spec/support/analytics_helpers.rb               — capture_analytics_events

Destination recommendation for the host wiring (separate PR in
coplan-square): a simple MySQL analytics_events table. Joins cleanly
to plans/users for every Cycle 2 KPI, ~1 day to wire, trivial to
dual-write to Snowflake later by extending the handler.

Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-17e7-738c-99b6-23e54fcd386f
Co-authored-by: Amp <amp@ampcode.com>
Copy link
Copy Markdown

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

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: 3ebb19f798

ℹ️ 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 on lines +84 to +86
if new_status == "considering" && old_status != "considering"
CoPlan::Analytics.track(
"plan_published",
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 Add plan_published tracking to API status updates

When a plan is published through the documented API metadata endpoint rather than this web-only update_status action, no plan_published event is emitted; I checked engine/app/controllers/coplan/api/v1/plans_controller.rb and its status-change path only logs the event and triggers reviews at lines 95-101. Agents are explicitly instructed to update status via the API, so publishing a brainstorm plan that way will undercount the core publish KPI even though the status transition succeeds.

Useful? React with 👍 / 👎.

Comment thread engine/app/models/coplan/comment.rb Outdated
comment_thread_id: comment_thread_id,
comment_id: id,
author_type: author_type,
is_first_in_thread: first_comment_in_thread?,
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 Stop using UUID order for first-comment analytics

For replies whose randomly generated UUID sorts before the existing thread comments, this new payload records is_first_in_thread: true even though the comment is not the first one; first_comment_in_thread? compares id < ?, but these IDs are UUID strings rather than insertion-ordered values. This makes comment_created analytics unreliable for replies and can skew any first-comment/thread-start counts derived from the event.

Useful? React with 👍 / 👎.

Two findings from codex on #116:

1. plan_published was only emitted from the web update_status action,
   missing every publish that flows through the documented API path at
   PATCH /api/v1/plans/:id. Since agent users are explicitly told to
   use the API, that was undercounting the core Cycle 2 publish KPI.
   Adds the same track call to the API controller, gated identically
   on the brainstorm→considering transition. Both call sites now tag
   the event with via: "web" or via: "api" so analytics can split
   or unify them as needed.

2. is_first_in_thread was derived from first_comment_in_thread?, which
   compares UUID strings with id < ? — UUIDs are not insertion-ordered,
   so a reply whose ID happens to sort below the existing comments
   would incorrectly record is_first=true. After-create-commit fires
   after the row is persisted, so a total count of 1 is the reliable
   signal. Inlined and commented.

   Pre-existing first_comment_in_thread? still has the same UUID bug
   for notify_plan_author — out of scope here; flagged separately to
   Hampton.

Specs: +3 new examples (API plan_published, UUID-out-of-order reply).
Full suite 890/890 green.

Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-17e7-738c-99b6-23e54fcd386f
Co-authored-by: Amp <amp@ampcode.com>
@HamptonMakes
Copy link
Copy Markdown
Collaborator Author

Addressed both Codex findings in 4941710:

  1. API plan_publishedPATCH /api/v1/plans/:id now also fires plan_published on the brainstorm→considering transition. Tagged both call sites with via: "web" / via: "api" so analytics can split or unify them.
  2. First-in-thread UUID bug — switched the analytics flag to comment_thread.comments.count == 1 (reliable after after_create_commit) instead of the existing first_comment_in_thread? helper, which compares UUIDs with id < ?. Inlined with a NOTE explaining why.

Pre-existing first_comment_in_thread? still has the same UUID-ordering bug for notify_plan_author — out of scope here; will flag separately.

Specs +3, full suite 890/890 green.

@HamptonMakes HamptonMakes merged commit 1c671e5 into main Jun 1, 2026
5 checks passed
@HamptonMakes HamptonMakes deleted the hampton/coplan-20/analytics-foundation branch June 1, 2026 21:53
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