feat: refactor + improve stability#1585
feat: refactor + improve stability#1585k11kirky wants to merge 1 commit into04-08-feat_improve_tests_fix_cache_and_fix_packagefrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
k11kirky
force-pushed
the
04-08-feat_improve_tests_fix_cache_and_fix_package
branch
from
8ec3c95 to
b27f7d7
Compare
k11kirky
force-pushed
the
04-09-feat_refactor_improve_stability
branch
from
b634893 to
7e80e97
Compare
| FeatureFlag, | ||
| } from "./types.js"; | ||
|
|
||
| export class PostHogApi { |
There was a problem hiding this comment.
is this something we can potentially share w/ the vs code extension too? not sure how valuable it is to share since just a wrapper on the API but wanna try to keep things tidy if/when we make the swap
There was a problem hiding this comment.
Yeah definitely — it's a thin wrapper around the PostHog REST API (flags, experiments, event definitions, event stats via HogQL). Same PostHogApi class could be shared with the VS Code extension. It'll ship as part of @posthog/enricher.
| enriched.enrichedFlags; | ||
| // [{ flagKey: "new-checkout", flagType: "boolean", staleness: "fully_rolled_out", | ||
| // rollout: 100, experiment: { name: "Checkout v2", ... }, ... }] | ||
|
|
||
| // Events with definition, volume, unique users | ||
| enriched.enrichedEvents; | ||
| // [{ eventName: "purchase", verified: true, lastSeenAt: "2025-04-01", |
There was a problem hiding this comment.
[the tiniest nitpick ever lol feel free to ignore]
i would prefer enriched.flags and enriched.events on this interface 🙈
There was a problem hiding this comment.
Already addressed in the simplify PR (#1589) — the README and EnrichedResult class both use enriched.flags and enriched.events 👍
| // { type: "flag", name: "new-checkout", flagType: "boolean", staleness: "fully_rolled_out", ... }] | ||
|
|
||
| // Source code with inline annotation comments | ||
| enriched.toComments(); |
| parts.push(`Experiment: "${flag.experiment.name}" (${status})`); | ||
| } | ||
| if (flag.staleness) { | ||
| parts.push(`STALE (${flag.staleness})`); |
There was a problem hiding this comment.
lazy-web: what are the possible values for flag.staleness ?
just wondering if it provides anything significantly different from the text "STALE", and if not, perhaps not work feeding to the LLM
There was a problem hiding this comment.
Values are: "fully_rolled_out" | "inactive" | "not_in_posthog" | "experiment_complete"
More granular than just "STALE" — the formatter shows it as STALE (fully_rolled_out) etc. Useful context for both agents and the diff viewer since you can distinguish between 'this flag doesn't exist in PostHog' vs 'this experiment is done' vs 'this is 100% rolled out for 30+ days'.
| // One comment per original source line — if multiple detections share a line, | ||
| // only the first (by sort order) gets an annotation to keep output readable. |
There was a problem hiding this comment.
thoughts on removing this restriction?
probably an edge case (i'd be impressed to see a single line of code with a large number of posthog events/flags 😛), but might be preferable to display everything, esp if the target audience is an agent?
(context for example: in the diff viewer, i'd like to add these things as token hovers, so we'd have full control over the UX and there's no issue with multiple things coming from a single src line)
There was a problem hiding this comment.
Good call — removed the per-line limit in the simplify PR (#1589). The current formatComments() inserts a comment for every item regardless. Agree this makes more sense especially for the token hover use case in the diff viewer.
k11kirky
force-pushed
the
04-09-feat_refactor_improve_stability
branch
from
7e80e97 to
bda15b3
Compare
k11kirky
force-pushed
the
04-08-feat_improve_tests_fix_cache_and_fix_package
branch
from
b27f7d7 to
8448ed6
Compare
2 participants
TLDR
Problem
Changes