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
| 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 🙈
| // { 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
| // 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)
2 participants
TLDR
Problem
Changes