[suggestion_engine] Handle new types of interactions for interruptions
Also adds a filter for suggestions that are interrupting at the moment.
This way we mark interrupting suggestions and don't show them in next
spaces.
This was causing issues when an interruption appears on top of a place
where next suggestions appear. Both were showing up in the same space.
We only want to show the interruption until an action is done on it.
MI4-1211 #comment Handle new types of interactions for interruptions
MI4-1211 #done
TESTED=CQ, suggestion_engine_test, affected unittests, device
Change-Id: I99346662ccbc1398ef340ad8e3327450a4473618
diff --git a/bin/suggestion_engine/BUILD.gn b/bin/suggestion_engine/BUILD.gn
index f881a19..900d8b7e 100644
--- a/bin/suggestion_engine/BUILD.gn
+++ b/bin/suggestion_engine/BUILD.gn
@@ -74,10 +74,12 @@
"//peridot/bin/suggestion_engine/decision_policies:rank_over_threshold_decision_policy_unittest",
"//peridot/bin/suggestion_engine/filters:conjugate_ranked_passive_filter_unittest",
"//peridot/bin/suggestion_engine/filters:ranked_active_filter_unittest",
+ "//peridot/bin/suggestion_engine/filters:ranked_passive_filter_unittest",
"//peridot/bin/suggestion_engine/rankers:linear_ranker_unittest",
"//peridot/bin/suggestion_engine/ranking_features:annoyance_ranking_feature_unittest",
"//peridot/bin/suggestion_engine/ranking_features:dead_story_ranking_feature_unittest",
"//peridot/bin/suggestion_engine/ranking_features:focused_story_ranking_feature_unittest",
+ "//peridot/bin/suggestion_engine/ranking_features:interrupting_ranking_feature_unittest",
"//peridot/bin/suggestion_engine/ranking_features:kronk_ranking_feature_unittest",
"//peridot/bin/suggestion_engine/ranking_features:mod_pair_ranking_feature_unittest",
"//peridot/bin/suggestion_engine/ranking_features:proposal_hint_ranking_feature_unittest",
diff --git a/bin/suggestion_engine/filters/BUILD.gn b/bin/suggestion_engine/filters/BUILD.gn
index 3db010a..94e59b3 100644
--- a/bin/suggestion_engine/filters/BUILD.gn
+++ b/bin/suggestion_engine/filters/BUILD.gn
@@ -9,6 +9,7 @@
":base",
":conjugate_ranked_passive_filter",
":ranked_active_filter",
+ ":ranked_passive_filter",
]
}
@@ -73,6 +74,36 @@
]
}
+source_set("ranked_passive_filter") {
+ sources = [
+ "ranked_passive_filter.cc",
+ "ranked_passive_filter.h",
+ ]
+
+ deps = [
+ ":suggestion_passive_filter",
+ "//peridot/public/fidl/fuchsia.modular",
+ ]
+
+ public_deps = [
+ "//peridot/bin/suggestion_engine/ranking_features:ranking_feature",
+ ]
+}
+
+executable("ranked_passive_filter_unittest") {
+ testonly = true
+
+ sources = [
+ "ranked_passive_filter_unittest.cc",
+ ]
+
+ deps = [
+ ":ranked_passive_filter",
+ "//garnet/public/lib/gtest",
+ "//third_party/googletest:gtest_main",
+ ]
+}
+
source_set("ranked_active_filter") {
sources = [
"ranked_active_filter.cc",
diff --git a/bin/suggestion_engine/filters/ranked_passive_filter.cc b/bin/suggestion_engine/filters/ranked_passive_filter.cc
new file mode 100644
index 0000000..36664c4
--- /dev/null
+++ b/bin/suggestion_engine/filters/ranked_passive_filter.cc
@@ -0,0 +1,26 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "peridot/bin/suggestion_engine/filters/ranked_passive_filter.h"
+
+#include <list>
+
+namespace modular {
+
+RankedPassiveFilter::RankedPassiveFilter(
+ std::shared_ptr<RankingFeature> ranking_feature)
+ : ranking_feature_(ranking_feature) {}
+
+RankedPassiveFilter::~RankedPassiveFilter() = default;
+
+// If the confidence of the ranking feature is 1.0 then this filter returns
+// true.
+bool RankedPassiveFilter::Filter(
+ const std::unique_ptr<RankedSuggestion>& ranked_suggestion) {
+ double confidence = ranking_feature_->ComputeFeature(
+ fuchsia::modular::UserInput(), *ranked_suggestion);
+ return confidence == kMaxConfidence;
+}
+
+} // namespace modular
diff --git a/bin/suggestion_engine/filters/ranked_passive_filter.h b/bin/suggestion_engine/filters/ranked_passive_filter.h
new file mode 100644
index 0000000..6833f9b
--- /dev/null
+++ b/bin/suggestion_engine/filters/ranked_passive_filter.h
@@ -0,0 +1,28 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PERIDOT_BIN_SUGGESTION_ENGINE_FILTERS_RANKED_PASSIVE_FILTER_H_
+#define PERIDOT_BIN_SUGGESTION_ENGINE_FILTERS_RANKED_PASSIVE_FILTER_H_
+
+#include <fuchsia/modular/cpp/fidl.h>
+
+#include "peridot/bin/suggestion_engine/filters/suggestion_passive_filter.h"
+#include "peridot/bin/suggestion_engine/ranking_features/ranking_feature.h"
+
+namespace modular {
+
+class RankedPassiveFilter : public SuggestionPassiveFilter {
+ public:
+ RankedPassiveFilter(std::shared_ptr<RankingFeature> ranking_feature);
+ ~RankedPassiveFilter() override;
+
+ bool Filter(const std::unique_ptr<RankedSuggestion>& suggestion) override;
+
+ private:
+ std::shared_ptr<RankingFeature> ranking_feature_;
+};
+
+} // namespace modular
+
+#endif // PERIDOT_BIN_SUGGESTION_ENGINE_FILTERS_RANKED_PASSIVE_FILTER_H_
diff --git a/bin/suggestion_engine/filters/ranked_passive_filter_unittest.cc b/bin/suggestion_engine/filters/ranked_passive_filter_unittest.cc
new file mode 100644
index 0000000..a16932e
--- /dev/null
+++ b/bin/suggestion_engine/filters/ranked_passive_filter_unittest.cc
@@ -0,0 +1,52 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "peridot/bin/suggestion_engine/filters/ranked_passive_filter.h"
+
+#include "gtest/gtest.h"
+#include "peridot/bin/suggestion_engine/ranking_features/ranking_feature.h"
+
+namespace modular {
+namespace {
+
+class ConfidenceRankingFeature : public RankingFeature {
+ public:
+ ConfidenceRankingFeature() {}
+
+ private:
+ double ComputeFeatureInternal(const fuchsia::modular::UserInput& query,
+ const RankedSuggestion& suggestion) override {
+ return suggestion.confidence;
+ }
+};
+
+class RankedPassiveFilterTest : public ::testing::Test {
+ public:
+ void SetUp() override {
+ filter = std::make_unique<RankedPassiveFilter>(
+ std::make_shared<ConfidenceRankingFeature>());
+ }
+
+ protected:
+ std::unique_ptr<RankedPassiveFilter> filter;
+};
+
+TEST_F(RankedPassiveFilterTest, FilterMaxConfidence) {
+ auto suggestion = std::make_unique<RankedSuggestion>();
+ suggestion->confidence = 1.0;
+ EXPECT_TRUE(filter->Filter(suggestion));
+}
+
+TEST_F(RankedPassiveFilterTest, FilterOtherConfidence) {
+ auto suggestion = std::make_unique<RankedSuggestion>();
+ suggestion->confidence = 0.5;
+ EXPECT_FALSE(filter->Filter(suggestion));
+
+ auto suggestion2 = std::make_unique<RankedSuggestion>();
+ suggestion2->confidence = 0.0;
+ EXPECT_FALSE(filter->Filter(suggestion2));
+}
+
+} // namespace
+} // namespace modular
diff --git a/bin/suggestion_engine/next_processor.cc b/bin/suggestion_engine/next_processor.cc
index a78e438..a654681 100644
--- a/bin/suggestion_engine/next_processor.cc
+++ b/bin/suggestion_engine/next_processor.cc
@@ -65,6 +65,7 @@
// TODO(miguelfrde): Make NextProcessor not depend on InterruptionsProcessor.
if (interruptions_processor_.MaybeInterrupt(*ranked_suggestion)) {
+ ranked_suggestion->interrupting = true;
debug_->OnInterrupt(prototype);
}
diff --git a/bin/suggestion_engine/ranked_suggestion.cc b/bin/suggestion_engine/ranked_suggestion.cc
index ea78dca..5c349b4 100644
--- a/bin/suggestion_engine/ranked_suggestion.cc
+++ b/bin/suggestion_engine/ranked_suggestion.cc
@@ -12,6 +12,7 @@
std::make_unique<RankedSuggestion>();
ranked_suggestion->prototype = prototype;
ranked_suggestion->hidden = false;
+ ranked_suggestion->interrupting = false;
return ranked_suggestion;
}
diff --git a/bin/suggestion_engine/ranked_suggestion.h b/bin/suggestion_engine/ranked_suggestion.h
index ebc99cd..fe7ab87 100644
--- a/bin/suggestion_engine/ranked_suggestion.h
+++ b/bin/suggestion_engine/ranked_suggestion.h
@@ -15,9 +15,20 @@
// sorted set of ranked suggestions, |rank| is increasing and
// |adjusted_confidence| is nonincreasing.
struct RankedSuggestion {
+ // Metadata of the suggestion.
SuggestionPrototype* prototype;
+
+ // Confidence of the suggestion. Updated during ranking.
double confidence;
+
+ // Whether or not the suggestion should be hidden (filtered by passive
+ // filters) in the list and not returned when fetching next suggestions.
bool hidden;
+
+ // Whether or not the suggesiton is currently being used as interruption. It
+ // should be filtered from the list that returns next suggestions.
+ bool interrupting;
+
static std::unique_ptr<RankedSuggestion> New(SuggestionPrototype* prototype);
};
diff --git a/bin/suggestion_engine/ranking_features/BUILD.gn b/bin/suggestion_engine/ranking_features/BUILD.gn
index 3994461..add3bf2 100644
--- a/bin/suggestion_engine/ranking_features/BUILD.gn
+++ b/bin/suggestion_engine/ranking_features/BUILD.gn
@@ -9,6 +9,7 @@
":annoyance_ranking_feature",
":dead_story_ranking_feature",
":focused_story_ranking_feature",
+ ":interrupting_ranking_feature",
":kronk_ranking_feature",
":mod_pair_ranking_feature",
":proposal_hint_ranking_feature",
@@ -84,6 +85,32 @@
]
}
+source_set("interrupting_ranking_feature") {
+ sources = [
+ "interrupting_ranking_feature.cc",
+ "interrupting_ranking_feature.h",
+ ]
+
+ deps = [
+ ":ranking_feature",
+ "//peridot/public/fidl/fuchsia.modular",
+ ]
+}
+
+executable("interrupting_ranking_feature_unittest") {
+ testonly = true
+
+ sources = [
+ "interrupting_ranking_feature_unittest.cc",
+ ]
+
+ deps = [
+ ":interrupting_ranking_feature",
+ "//garnet/public/lib/gtest",
+ "//third_party/googletest:gtest_main",
+ ]
+}
+
source_set("kronk_ranking_feature") {
sources = [
"kronk_ranking_feature.cc",
diff --git a/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.cc b/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.cc
new file mode 100644
index 0000000..2aedf41
--- /dev/null
+++ b/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.cc
@@ -0,0 +1,22 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "peridot/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.h"
+
+namespace modular {
+
+InterruptingRankingFeature::InterruptingRankingFeature() {}
+
+InterruptingRankingFeature::~InterruptingRankingFeature() = default;
+
+double InterruptingRankingFeature::ComputeFeatureInternal(
+ const fuchsia::modular::UserInput& query,
+ const RankedSuggestion& ranked_suggestion) {
+ if (ranked_suggestion.interrupting) {
+ return kMaxConfidence;
+ }
+ return kMinConfidence;
+}
+
+} // namespace modular
diff --git a/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.h b/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.h
new file mode 100644
index 0000000..7509ea9
--- /dev/null
+++ b/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.h
@@ -0,0 +1,26 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PERIDOT_BIN_SUGGESTION_ENGINE_RANKING_FEATURES_INTERRUPTING_RANKING_FEATURE_H_
+#define PERIDOT_BIN_SUGGESTION_ENGINE_RANKING_FEATURES_INTERRUPTING_RANKING_FEATURE_H_
+
+#include <fuchsia/modular/cpp/fidl.h>
+
+#include "peridot/bin/suggestion_engine/ranking_features/ranking_feature.h"
+
+namespace modular {
+
+class InterruptingRankingFeature : public RankingFeature {
+ public:
+ InterruptingRankingFeature();
+ ~InterruptingRankingFeature() override;
+
+ private:
+ double ComputeFeatureInternal(const fuchsia::modular::UserInput& query,
+ const RankedSuggestion& suggestion) override;
+};
+
+} // namespace modular
+
+#endif // PERIDOT_BIN_SUGGESTION_ENGINE_RANKING_FEATURES_INTERRUPTING_RANKING_FEATURE_H_
diff --git a/bin/suggestion_engine/ranking_features/interrupting_ranking_feature_unittest.cc b/bin/suggestion_engine/ranking_features/interrupting_ranking_feature_unittest.cc
new file mode 100644
index 0000000..7b8a926
--- /dev/null
+++ b/bin/suggestion_engine/ranking_features/interrupting_ranking_feature_unittest.cc
@@ -0,0 +1,35 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "peridot/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.h"
+
+#include "gtest/gtest.h"
+
+namespace modular {
+namespace {
+
+class InterruptingRankingFeatureTest : public ::testing::Test {
+ protected:
+ InterruptingRankingFeature ranking_feature;
+ fuchsia::modular::UserInput query;
+};
+
+TEST_F(InterruptingRankingFeatureTest, ComputeFeatureInterrupting) {
+ RankedSuggestion suggestion;
+ suggestion.interrupting = true;
+
+ double value = ranking_feature.ComputeFeature(query, suggestion);
+ EXPECT_EQ(value, 1.0);
+}
+
+TEST_F(InterruptingRankingFeatureTest, ComputeFeatureNonInterrupting) {
+ RankedSuggestion suggestion;
+ suggestion.interrupting = false;
+
+ double value = ranking_feature.ComputeFeature(query, suggestion);
+ EXPECT_EQ(value, 0.0);
+}
+
+} // namespace
+} // namespace modular
diff --git a/bin/suggestion_engine/suggestion_engine_impl.cc b/bin/suggestion_engine/suggestion_engine_impl.cc
index 08697d3..450af0f 100644
--- a/bin/suggestion_engine/suggestion_engine_impl.cc
+++ b/bin/suggestion_engine/suggestion_engine_impl.cc
@@ -19,10 +19,12 @@
#include "peridot/bin/suggestion_engine/decision_policies/rank_over_threshold_decision_policy.h"
#include "peridot/bin/suggestion_engine/filters/conjugate_ranked_passive_filter.h"
#include "peridot/bin/suggestion_engine/filters/ranked_active_filter.h"
+#include "peridot/bin/suggestion_engine/filters/ranked_passive_filter.h"
#include "peridot/bin/suggestion_engine/rankers/linear_ranker.h"
#include "peridot/bin/suggestion_engine/ranking_features/annoyance_ranking_feature.h"
#include "peridot/bin/suggestion_engine/ranking_features/dead_story_ranking_feature.h"
#include "peridot/bin/suggestion_engine/ranking_features/focused_story_ranking_feature.h"
+#include "peridot/bin/suggestion_engine/ranking_features/interrupting_ranking_feature.h"
#include "peridot/bin/suggestion_engine/ranking_features/kronk_ranking_feature.h"
#include "peridot/bin/suggestion_engine/ranking_features/mod_pair_ranking_feature.h"
#include "peridot/bin/suggestion_engine/ranking_features/proposal_hint_ranking_feature.h"
@@ -191,20 +193,34 @@
auto& proposal = suggestion->prototype->proposal;
auto proposal_id = proposal.id;
auto preloaded_story_id = suggestion->prototype->preloaded_story_id;
- if (interaction.type == fuchsia::modular::InteractionType::SELECTED) {
- if (preloaded_story_id.empty()) {
+ suggestion->interrupting = false;
+ switch (interaction.type) {
+ case fuchsia::modular::InteractionType::SELECTED: {
+ if (!preloaded_story_id.empty()) {
+ // Notify before we promote + remove from next since the proposal
+ // won't be available when that happens.
+ if (proposal.listener) {
+ auto listener = proposal.listener.Bind();
+ listener->OnProposalAccepted(proposal.id, preloaded_story_id);
+ }
+ PromoteNextProposal(component_url, preloaded_story_id, proposal_id);
+ // Only next suggestions can be rich so we can finish early.
+ // No need to remove.
+ return;
+ }
PerformActions(std::move(proposal.on_selected),
std::move(proposal.listener), proposal.id,
proposal.story_name, component_url, proposal.story_id);
- } else {
- // Notify before we promote + remove from next since the proposal
- // won't be available when that happens.
- if (proposal.listener) {
- auto listener = proposal.listener.Bind();
- listener->OnProposalAccepted(proposal.id, preloaded_story_id);
- }
- PromoteNextProposal(component_url, preloaded_story_id, proposal_id);
- // Only next suggestions can be rich so we can finish early.
+ break;
+ }
+ case fuchsia::modular::InteractionType::DISMISSED: {
+ break; // Remove suggestion since it was dismissed by user.
+ }
+ // These actions .
+ case fuchsia::modular::InteractionType::EXPIRED:
+ case fuchsia::modular::InteractionType::SNOOZED: {
+ // No need to remove since it was either expired by a timeout in
+ // user shell or snoozed by the user.
return;
}
}
@@ -272,6 +288,8 @@
std::make_shared<AnnoyanceRankingFeature>();
ranking_features["dead_story_rf"] =
std::make_shared<DeadStoryRankingFeature>();
+ ranking_features["is_interrupting_rf"] =
+ std::make_shared<InterruptingRankingFeature>();
// Get context updates every time a story is focused to rerank suggestions
// based on the story that is focused at the moment.
@@ -316,6 +334,8 @@
std::vector<std::unique_ptr<SuggestionPassiveFilter>> passive_filters;
passive_filters.push_back(std::make_unique<ConjugateRankedPassiveFilter>(
ranking_features["focused_story_rf"]));
+ passive_filters.push_back(std::make_unique<RankedPassiveFilter>(
+ ranking_features["is_interrupting_rf"]));
next_processor_.SetPassiveFilters(std::move(passive_filters));
}
diff --git a/public/fidl/fuchsia.modular/suggestion/suggestion_provider.fidl b/public/fidl/fuchsia.modular/suggestion/suggestion_provider.fidl
index c91711b..a295b5b 100644
--- a/public/fidl/fuchsia.modular/suggestion/suggestion_provider.fidl
+++ b/public/fidl/fuchsia.modular/suggestion/suggestion_provider.fidl
@@ -99,9 +99,25 @@
};
enum InteractionType {
+ // Set when a suggestion was accepted by the user. In the case of an
+ // interruption, it won't be placed in the next suggestions.
SELECTED = 0;
+
+ // Set when a suggestion was removed by the user. In the case of an
+ // interruption, it won't be placed in the next suggestions and in the case of
+ // a non-interruptive suggestion it will be removed from it.
DISMISSED = 1;
- // SNOOZED
+
+ // Set when a suggestion was hidden by the user. An interruption that is
+ // snoozed will be placed in next.
+ // Note: as of 08/03/2018 this interaction is only defined for interruptions.
+ // TODO(miguelfrde): define for regular suggestions.
+ SNOOZED = 2;
+
+ // Set when a suggestion is hidden by the user shell with no user interaction.
+ // An interruption that is expired is placed in the next space.
+ // This has no effect on regular suggestions, only on interruptions.
+ EXPIRED = 3;
};
struct Interaction {
