Filter problems and summarize recent practice - #108
Danielncku wants to merge 3 commits into
Conversation
jserv
left a comment
There was a problem hiding this comment.
Check https://cbea.ms/git-commit/ carefully and enforce the rules.
ColtenOuO
left a comment
There was a problem hiding this comment.
Could you also include a commit body? It will make it much easier to understand the changes when tracking them down in the future.
058eaec to
4d9caf7
Compare
|
Addressed the requested changes in 4d9caf7:
Verification:
The complete Windows run executed 627 tests: 608 passed, 5 skipped, and 14 unrelated pre-existing platform/vendor failures remained (Windows ESM paths and CRLF-sensitive checks, plus missing MediaPipe vendor assets). |
4d9caf7 to
eabeacd
Compare
|
|
||
| function showProgress(entries, suffix) { | ||
| renderPracticeFocus(); | ||
| renderRecentPerformance(); |
There was a problem hiding this comment.
erasable just below is entries.length > 0 || readDeviceHistory().length > 0, so a candidate whose only local practice data is recorded starts gets #history-header hidden and no Delete control, while this call still prints 1 recent interview start saved locally. That makes the new store the one thing the lobby writes to the device that the candidate cannot erase. Count practiceAttempts.length in erasable.
There was a problem hiding this comment.
practiceAttempts.length is now part of the erasable condition. A start-only browser test confirms that Delete is visible, removes the active and anonymous start data, and preserves another signed-in account scope.
|
Hi @jserv, thank you very much for taking the time to provide such a detailed review. I have replied in each thread with my current understanding and proposed direction. As I am still learning the project’s design conventions, I would like to make sure my interpretation is correct before changing the implementation. Two details—the deletion boundary across local scopes and the count-based incomplete model—seem especially worth confirming first. I will therefore hold off on pushing another revision for now. Once the intended behavior is clear, I will rebase onto the latest main, resolve the conflicts carefully, implement the agreed changes together, run the full relevant test suite, and then update this pull request. Please feel free to correct any part of my understanding. |
Always rebase onto the latest Don't have to say greeting. Focus on efficient discussions. |
eabeacd to
62b5bd7
Compare
|
Rebased onto upstream/main at 42c3b08 before applying the review changes, resolved the app.js conflicts, and updated this PR with commit 62b5bd7. The ten inline issues have been addressed and replied to individually. Validation completed: 86 focused Node tests passed; 8 targeted Playwright regression scenarios passed; ESLint, JavaScript syntax checks, generated-card consistency, and git diff checks passed. I will use the same sync-first workflow for any further update. |
|
Please rebase onto the latest main and resolve any conflicts. |
8e99d68 to
a45ce6e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
jserv
left a comment
There was a problem hiding this comment.
Rebase the current branch onto the upstream default branch and rework the series into functionally minimal commits, folding similar ones and enforcing the project's commit message rules.
a45ce6e to
763a0ba
Compare
|
I narrowed this pull request to topic filtering and a recent practice snapshot. The topic selection now combines with the existing difficulty filter and stays local to the browser; the snapshot counts only dated The earlier incomplete-start and account-scoping changes have been removed. Focused unit and browser tests cover filtering, snapshot calculation, and report-load failure handling. |
ColtenOuO
left a comment
There was a problem hiding this comment.
Check the CI error logs and fix the issues.
763a0ba to
ac27118
Compare
| if (manualProblem) return; | ||
| const choice = pickProblem( | ||
| cards, | ||
| topicEligibleCards(), |
There was a problem hiding this comment.
Passing the topic-narrowed pool into pickProblem changes two things the description does not mention. Due reviews outside the selected topic are never offered, even though the difficulty filter deliberately lets a review through ("A review can fall outside the levels now selected"), and the topic persists across visits. And once every problem in the topic is passed, choice.repeat produces "You have passed every problem at this level." while unpassed problems at that level remain in other topics. Keep due drawn from all cards, as the difficulty path does, and qualify the repeat message with the topic when one is selected.
There was a problem hiding this comment.
sorry i didnt find the due review. i already rebased the newest.
1.Due reviews are now selected from the complete problem set, independently of the topic-filtered visible pool, so an overdue review keeps its existing highest priority!
2.When every problem matching the selected topic and level has been passed, the message now says: “You have passed every problem matching this topic and level.” Both behaviors are covered by browser tests.
Expose problem topics as local card metadata so candidates can narrow the lobby without revealing tags on each card. Keep recommendations inside the selected topic and cover combined filters in unit and browser tests.
Show a compact snapshot for the latest assessed interviews with pass rate, current streak, and topics that have more misses than passes. Hide the snapshot when history is unavailable and cover both success and failure paths.
ac27118 to
ed665f6
Compare
Keep problem topic metadata out of the initial markup and load it only when the candidate opens the picker. Reset topic selection on restore and keep manual choices aligned with the combined filters. Preserve due-review priority across topic filters, exclude incomplete reports from recent performance, and add regression coverage for the reviewed cases.
ed665f6 to
603676f
Compare
|
I will fix this CI later |
What this does
The lobby already lets candidates choose a difficulty, but it offers no way to narrow the problem bank by topic. This change adds a local topic selector that combines with the existing difficulty checkboxes and limits both the visible cards and the recommendation pool. The selection is remembered in local storage.
Problem topics are generated from the existing problem bank and stored only as card metadata. They are not rendered on the cards, so the scenario does not reveal the underlying problem category.
The lobby also shows a compact snapshot of the latest eight assessed interviews. It reports passes, misses, pass rate, the current pass or miss streak, and topics with more misses than passes. Only dated reports with an explicit
HIREorNO_HIREdecision are counted. The snapshot stays hidden when there is no assessed history or report loading fails.Not included
This version does not track interview starts, incomplete sessions, account-specific local state, or a Failed Before filter. Those earlier changes were removed to keep the pull request focused on topic selection and report-derived insights.
Validation
Focused unit tests cover topic extraction, combined difficulty and topic filtering, recent-result ordering, pass-rate calculation, unscored reports, and weak-topic detection.
Playwright tests cover filtering and reset behavior, rendering the recent snapshot from assessed reports, and hiding it when report history cannot be loaded.
ESLint, Prettier, and the generated problem-card consistency check pass locally. The lobby and
/interviewroutes both return 200 from the official Windows Rust server with this checkout'sweb/override.The full browser suite is not green on this Windows checkout: 16 failures remain in avatar, vendor, and source-policy checks outside the changed paths, so this description does not claim a fully green local gate.