This involves declaring the 20220603-variant-a the winner of the past
experiment; note in the past experiment there was not a statistically
significant winner.
This new experiment adds a variant that deviates from the new
incumbentby way of reintroducing the follows an author and adjusting the
tag follow weights.
Closesforem/forem#18082
```shell
$ diff config/feed-variants/20220617-variant-a.json \
config/feed-variants/20220712-variant.json
```
Results in:
```shell
3c3
< "description": "Blend of 20220603-variant-b and 20220603-variant-a",
---
> "description": "Builds from 20220617-variant-a (see https://github.com/forem/forem/issues/18082)",
50a51,57
> "following_author": {
> "cases": [
> [0, 0.8],
> [1, 1]
> ],
> "fallback": 0.8
> },
70,73c77,80
< [0, 0.5],
< [1, 0.85],
< [2, 0.9],
< [3, 0.95],
---
> [0, 0.3],
> [1, 0.9],
> [2, 0.95],
> [3, 0.98],
```
This change introduces several things things:
1. Fixes the performance of the `/admin/abtests` page; instead of
rendering all of the experiment results, just render the overview.
2. Adjust the show page for an experiment to show the summary of
results.
3. Favor the existing "started_at" and "ended_at" attributes of
FieldTest (see https://github.com/ankane/field_test#config)
4. Update the field test experiments to include the `started_at` and
`ended_at` attributes.
5. Skip processing all non-active experiments in our conversion handler.
Why are there no tests? This is a page that is Admin only and is only
visible for reporting purposes. So, it's not quite worth writing tests
as you'd need lots of data. So we'll assume the logic from the upstream
[field_test gem][1] is adequate.
Closesforem/forem#17869
[1]:https://github.com/ankane/field_test
This commit contains two separate but related changes:
1. A check-list for creating a new experiment.
2. A new experiment along with declaring a winner for the previous experiment.
Below are the diffs of the two new variants versus the incumbent for the
experiment:
```
❯ diff config/feed-variants/20220603-variant-b.json config/feed-variants/20220518-variant.json
3,4c3
< "description": "As 202205518-variant but with modificiation to `order_by` lever.",
< "order_by": "final_order_by_random_weighted_to_score",
---
> "order_by": "relevancy_score_and_publication_date",
```
```
❯ diff config/feed-variants/20220603-variant-a.json config/feed-variants/20220518-variant.json
3d2
< "description": "As 202205518-variant but with modificiation to `matching_positive_tags_intersection_count`.",
69c68
< [0, 0.5],
---
> [0, 0.7],
```
Closesforem/forem#17822
Prior to this commit, as a matter of undocumented practice, we would
overwrite past experiments with upcoming experiments.
With this commit, we're leveraging a feature of the field_test gem:
declaring a winner. Instead of overwriting an experiment with the next
experiment, we're going to declare the expiring experiment's winner then
prepend to the ./config/field_test.yml the new experiment.
The [field_test/app/views/field_test/experiments/index.html.erb][1] then
uses logic to first render "active" experiments and "completed"
experiments. A "completed" experiment is one in which we've declared a
winner.
Closesforem/forem#17816
[1]:https://github.com/ankane/field_test/blob/master/app/views/field_test/experiments/index.html.erb
With comments, publishing articles, and reactions we had symmetry on two
goals:
- Create a _subject_ (e.g. Comment, Published Article, Reaction)
- Create four _subjects_ within a week.
For page views we only had "Create four _subjects_ within a week." This
PR addes the "Create a _subject_".
To do this required adjusting some tests as they were too specific in
nature (looking at an expected count).
In addition, in consultation with Jennie, I've updated the experiment
order to better reflect some hierarchical importance.
**Rollback considerations:**
This also includes a feature flag that we can explicitly disable if we
overload the application with workers handling reaction goals. Using
`FeatureFlag.accessible?(:field_test_event_single_create_pageview)`
returns `true` unless we explicitly disable this flag.
Related to forem/forem#17673
Related to forem/forem#17669Closesforem/forem#17691
This commit adds three new feed experiment goals:
* user publishes four posts within a week
* user reacts with a "heart", "unicorn", or "reading list" to an article
* user reacts with a "heart", "unicorn", or "reading list" to an article
four times in a week
In addition it adds it to the existing experiment. Adding it to the
existing experiment is acceptable because:
1. We can ignore the results
2. The experiments are structured such that these new goals could be met
with prior data.
3. Both variants are playing by the same rules, so the results relative
to each other are valid.
**There is a nuanced assumption in how we handle reactions:**
This implementation does not count by unique article reactions. However,
most folks will ❤️ , 🦄 , and 🔖 in one swoop; thus
those 3 reactions are all grouped into happening on one day.
Further, it's a reflection of the person taking an action after reading
a post, not how much action on that particular post.
**Rollback considerations:**
This also includes a feature flag that we can explicitly disable if we
overload the application with workers handling reaction goals. Using
`FeatureFlag.accessible?(:field_test_event_for_reactions)` returns
`true` unless we explicitly disable this flag.
Closesforem/forem#17669
* Adding new relevancy lever for privileged reactions
This PR includes four changes:
1. Renaming the variant to remove Jennie's name
2. Starting a new experiment
3. Adding a new relevancy lever
4. Adding a new variant that uses the relevancy lever
1 and 2 is somewhat straight-forward.
For 3, we're looking at having a total of 5 "slots" for privileged user
reactions. Hence the four boundary values.
For 4, we're configuring the granular levers with the following:
- `[-∞..-5)` should have a 0.05
- `[-5..0)` should have a 0.5
- `[0..5)` should have a 0.9
- `[5..10)` should have a 0.98
- `[10..∞)` should have a 1.0
The 20220509-variant.json is a copy of 20220422-variant.json but
replaces the `privileged_user_reaction` lever with the
`privileged_user_reaction_granular` lever.
Below is the diff to highlight the similarities and differences between
the two variants.
```shell
❯ diff config/feed-variants/20220509-variant.json config/feed-variants/20220422-variant.json
90c90
< "privileged_user_reaction_granular": {
---
> "privileged_user_reaction": {
92,96c92,93
< [-2, 0.05],
< [-1, 0.5],
< [0, 0.9],
< [1, 0.98],
< [2, 1]
---
> [-1, 0.2],
> [1, 1]
98,102c95,97
< "fallback": 0.9,
< "very_negative_reaction_threshold": -5,
< "negative_reaction_threshold": 0,
< "positive_reaction_threshold": 5,
< "very_positive_reaction_threshold": 10
---
> "fallback": 0.95,
> "negative_reaction_threshold": -10,
> "positive_reaction_threshold": 10
```
Closesforem/forem#17584
* Update app/models/articles/feeds.rb
Co-authored-by: Josh Puetz <josh@dev.to>
* Adjusting testing logic to reflect latest experiment
* Adjusting negative threshold
Co-authored-by: Josh Puetz <josh@dev.to>
* Adding a field test option for post publishing goals
We have page view and comment view "goals" but have not looked at
publication "goals". We're using these "goals" to help us refine the
feed towards sustainable engagement and community health.
The conjecture on publication goals is that the content on the feed
itself can nudge folks in their decision to publish posts.
Closesforem/forem#17585
* Fixing broken test
* Moving Experiments into AbExperiment namespace
This commit entails two major concepts:
1. Extracting logic out of a worker.
2. Cleaning the conditions in which we convert experiments.
In addition, I revisited some spec names to tidy them up for
clarification and structure.
**Extracting Logic...**
For workers, my preference is that they be a simple router to another
object. A non-worker object, with it's initialize method, can both
better manage instance variables. Further, workers are somewhat "flat"
in hierarchy,
In this case, having the AbExperiment be knowledgeable both in setting
the experiment and handling conversions makes (to me) organizational
sense. For example, the AbExperiment is constructed as a wrapper to the
FieldTest gem. But the RecordFieldTestEventWorker had knowledge of
FieldTest.
With this refactor, it does not have that knowledge. This also provided
an opportunity to replace magic strings with constants.
**Cleaning the conditions...**
This relates directly to and closesforem/forem#17530. I was looking at
the experiments after 2 days, and realized that there shouldn't be
results for
`user_views_pages_on_at_least_nine_different_days_within_two_weeks`;
after all the experiment started 2 days ago, how can we have results
that look at 2 weeks.
By adding the "max" function calls, we ensure a clear boundary of
"before the experiment began" versus "while the experiment is running".
Closesforem/forem#17530
* Update spec/models/ab_experiment_spec.rb
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
* Apply suggestions from code review
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
This commit modifies two things:
1. The sort order of the feed.
2. Locking in the next incumbent for the feed.
In consultation with Jamie regarding the SQL, I've switched from a WHERE
IN type clause to creating a JOIN. This allows for the calculated
relevancy score to become a sortable value for the feed.
Second the incumbent feed is a slight modification of the past winner;
based on the results of a long running experiment. This adjustment is
per conversations with product.
The past experiment ran since early February, so it's time to retire it,
regroup and move forward.
Closesforem/forem#17307
* Initial work
* Implement strategy injection into WeightedQueryStrategy
* Modify field_test config and adding variants
* Change orginal to constant and make some other adjustments
* Fix hardcoded test values
* Proposing a new feed experiment
This commit involves tweaking a few subtle aspects of the challenger:
1) Remove the sort by published date on the relevance score. The
articles will now be listed in the order of perceived relevance.
2) Disable a few levers that did little. There really aren't any
`articles.featured = true` articles in the database.
3) Gently increase the weight of each comment in a linear manner.
4) Give a little more weight to posts that don't have followed tags.
In addition, it involves renaming the conversions to better convey their
implementation. This renaming helps create more descriptive labels for
the test results at `/admin/abtests`.
This commit also adds logic to repurpose the AbExperiment feed_strategy
logic; I envision adjusting the weighted feed strategy levers with some
frequency.
Per discussions, I'm also disabling the "not logged in" feed testing.
We'll use the LargeForemExperimental for this.
* Flipping attribute name to positive
Mentally "not disabled" is harder to parse than "enabled". This change
helps with setting an optimistic attribute.
* Bump for travis
* Bump for travis
* Removing file committed by mistake
* Extracting method
* AB Test for Feed Strategy
Adding the plumbing for performing AB Tests on the feed strategy.
You can add the environment variable `AB_TEST_SERVICE_FEED_STRATEGY` to
specify the field test to run.
For example, from the forem application home directory you can run the
following to force a strategy for all users:
```console
AB_TEST_SERVICE_FEED_STRATEGY=original bin/start
```
Forcing the strategy can significantly help with quality assurance testing.
* Adding configurable weighted feed strategy
I wrote about this exploration on [DEV.to][1]. This particular
implementation provides significant configuration options for A/B
testing.
At it's core this `Articles::Feeds::WeightedQueryStrategy` provides a
means to query articles that are relevant to the user. The
configuration helps us test and refine those values.
Further more this implementation introduces the idea of some common
interfaces for feed strategies. I'm not happy with the naming
convention but we can work to change that.
This even hints at a mechanism for administrators to configure.
Related to forem/rfcs#314, as I work towards understanding the feed
algorithm. And #15180
[1]:https://dev.to/jeremyf/practicing-postgresql-and-postulating-improvements-5m
* Renaming AbTestService to AbExperiment
I'm loath to end any production class's filename with "test". I don't
want minitest or rspec coming along and attempting to "test" that file.
In addition, I find that Experiment better describes the concept.
Also, per conversations with Michael K, we favor nouns objects to go
in app/models and verb objects to go in app/services. That's a great
separation.
* Factoring AbExperiment into a Controller concern
I wish this could be a stand-alone module but the implementation of
field test has cases that require controller context.
* Sanitizing configurable weighted strategy cases
* Minor refactoring of coding structure
* Renaming parameter for clarity
* Renaming variable to avoid confusion
Prior to the commit, `env` is a controller variable. The injected `env`
variable didn't collide, but did create just a bit of confusion.
With this change, I'm trying to disentangle from that variable.
* Factoring AbExperiment away from mixin concern
Per discussion with Michael K, we both agree that we do not like
controller concern mixins. They can quickly complicate the legibility
of any given codebase.
This refactor provides a common and simplified interface for a Plain Old
Ruby Object (PORO) that accounts for the implementation considerations
of the underlying "field_test" controller method.
* Adding Datadog.tracer.trace around feed actions
* Adding featured_article_factor
Prior to this commit, there was no consideration for how moderators
engage with an article. This is a first nudge towards that effort.
* Removing spec
Add a spec that says yes or no we're feature testing something seems
fragile. Namely, why are we wanting to assert that as a truism.
* Helping ensure we're "tracing" the same things
From the code comment:
> Hey, why the to_a you say? Because the
> LargeForemExperimental has already done this. But the
> weighted strategy has not. I also don't want to alter the
> weighted query implementation as it returns a lovely
> ActiveRecord::Relation. So this is a concession.
* Addressing parameters for tracing function
* For weight feed query only count positive tags
A user assigns explicit points to the tags that they do or do not want
to see. This change treats the negative explicit_points as an indicator
that they don't want to see the tag.
* Adding weighted factor for privileged user reactions
* Updating comments to better clarify
* Adding additional constraint for published_at
* Updating documentation and adding logger
* Adding documentation concerning method call
* Upping logging level for experiments
See conversations in https://github.com/forem/forem/pull/15240
* Adding group by clauses to appease postgres configs
* Injecting relevant scoring method's joins
Prior to this commit, we included all of the joins regardless of the
scoring methods chosen. With this commit, we only inject joins that are
used with the commit.
What this would allow us to do is to then have some pre-cooked scoring
methods but not turn them on. Or to turn off the more expensive scoring
methods.
* Adding feature flag for feed strategy experiment
* Declare winner in feed_top_articles_query test
* Some WIP modifications to lfe
* Fix up tests and generalize
* Fix style
* Adjust tests to handle different field test scenarios
* Remove socre_randomness tests
* Fix test to fit stub
* Update app/workers/users/record_field_test_event_worker.rb
