Fix #4652 : Implemented Learner Progress for Exploration State

[Rd4dev]

Fixes #4652

Explanation

• This PR provides an implementation for displaying learning progress in the exploration lessons.
• As mentioned in the issue, Oppia lessons are dynamic in nature, and to define the progress, the flow of lessons is mandatory.
  • One way we could fix this is by providing order numbers to the JSON files themselves, but that would make it difficult to scale and add other states in the future.
  • So, I implemented a small parser that identifies the order of the lessons by going through the JSON.

• The way the parser works is as follows:
  • It starts off with init_state_name.
  • With each state, it first identifies its default_outcome and assigns a position to them.
  • Then, it checks the answers_group from bottom to top and assigns those states their positions one by one (note that only the first entries are noted).
  • Thereby, the order of the exploration lessons is obtained. I have attached the pseudocode for reference and the obtained list of statename : positions below.

• The way it handles the dynamic changes of questions is that the total number of questions remains the same as the total number of states. The current state is checked on every new exploration. Let's say we have 3 states;
  • answering state 1 correctly will skip you to state 3, making the progress jump from 10% to 30%.
  • If state 1 is answered incorrectly, the result_answers are checked, and state 2 is triggered, making the progress go from 10% to 20%.
  • The same applies to the decrement of progress when recapping back to previous states.
• The progress bar uses the same design as the question player progress bar.

TODO:

• runBlocking should be properly checked and handled if needed, replaced with proper coroutine.
• Tests will be updated soon. -[Hindered Implementation due to Bug [BUG]: TestCoroutineDispatchers time out sometimes in large Espresso Tests #5379
• Check landscape implementation
• To check visibility and proper positioning with audio block

Note:

• This is a proposed solution. I would love to hear feedback on whether this would be viable or not.

Essential Checklist

• The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• Any changes to scripts/assets files have their rationale included in the PR explanation.
• The PR follows the style guide.
• The PR does not contain any unnecessary code changes from Android Studio (reference).
• The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
• The PR is assigned to the appropriate reviewers (reference).

Demonstration

StateProgress3.mp4

[Rd4dev]

@BenHenning, @adhiamboperes Would this approach be effective?

[oppiabot]

Hi @Rd4dev, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[Rd4dev]

@adhiamboperes PTAL

[adhiamboperes]

Thanks @Rd4dev! This approach is really intersting. I'm handing over to @BenHenning to verify the approach, and @seanlip to look over the progressbar design for Product.

As a workaround to the Espresso run issues, I usually modify the test setup to unregister IdlingResource immediately after the profiles have been setup in the setUpTest() Function. This should allow you to write tests.

[seanlip]

THis is quite interesting, thanks @Rd4dev!

From the product side, I don't have an objection, it would actually not be a bad idea to test this. The only request I have is to put this functionality behind a feature flag so that we can do some controlled testing before launching it more broadly.

The only other concern I have is that it's a bit weird how the content gets "sucked up" into the progress bar -- I'm not sure about the handling of the cutoff when scrolling. Would you mind filing an issue on the design team repo to take a look at this, and I will talk to the Android design team to see if they can look at it? This doesn't need to block the merge of this PR though (assuming that the functionality remains gated behind a feature flag).

Thanks!

[Rd4dev]

Thank you for the initial approval, @seanlip, I've filed a new issue in the design team repository.
Will reserve the feature under a feature flag in the upcoming commits.

[BenHenning]

Thanks @Rd4dev! I moved this over to draft because I think it has a lot of iteration needed before it's actually ready to be finalized and go into full review. I left one main comment to analyze the algorithm first--once we finalize that, I'll have more comments on how to model the dataflow and API changes. After that's done, the final steps are focusing on the UI and testing side of things.

This PR will probably require a lot of back-and-forths to finalize given the nature of this problem: modeling progress is complex in Oppia lessons since they have non-linear paths that are potentially unique to an individual's learning experience. We've fortunately thought about this in some detail, but if you're unsure whether you'll have the time to finish this PR let me know and we can move the discussion for the algorithm over to the issue for whoever works on it next.

[BenHenning]

@seanlip has anyone worked on the in-lesson progress bar on web? I'm wondering if anyone has already come up with an algorithm for computing the linear path through the lesson that we could copy here for parity.

@Rd4dev one potential issue I see with this approach is that it's sort of implicitly assuming that each destination is an equal outcome when making progress. So long as we never let the user go backwards in progress, that helps a bit with making sure the progress bar looks correct, but it may be a bit unpredictable since sometimes states with many outcomes will show "more progress" of completing than those with few.

I think the intended way to implement this is to use a pathfinding algorithm from the starting state to all ending states. There should be only one path (but we'll need some sort of tiebreaker if there is more than one). From there, we only count progress for states who have checkpoints marked (this is a property on Oppia web we don't currently import: https://github.com/oppia/oppia/blob/5ebbb3d369532a22ee1638a810d597ffb9719594/core/domain/state_domain.py#L3615).

@seanlip does the algorithm explained in the above paragraph sound correct to you (in the event there isn't an existing implementation to follow)? Also, is the checkpoint field being set in lessons now?

[seanlip]

@BenHenning Yes, we have done a bit of work here. We label some states as checkpoints, and use those checkpoints in the lessons as a measure of progress. The checkpoint states are always located at articulation points of the graph (i.e. if you remove the checkpoint state then it disconnects the graph). Lessons have 2-8 checkpoints and progress is measured by the achievement of checkpoints. More granular progress is not recorded.

The checkpoint field is being set in lessons, and the third paragraph you mention seems like a good approach to follow. It does match what we do on Web. You don't need a tiebreaker, given the "articulation points" property above.

Please let me know if you need more info. Also, I apologize for entirely missing this in my previous comment -- for some reason I didn't make the connection between the progress bar and the existing checkpoints functionality on Web.

[oppiabot]

Hi @Rd4dev, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[oppiabot]

Hi @Rd4dev, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[BenHenning]

@Rd4dev are you planning on working on this? Otherwise, I think it might be best if we close it and unassign the issue so that others can take it up if they have availability.

It sounds based on Sean's latest feedback that we'd need to update the import asset script & domain protos for lessons to pull in the checkpoint properties so that we have a baseline for computing progress through an exploration.