Skip to content

feat: added onboarding#243

Open
null-crafter wants to merge 3 commits into
mainfrom
feat/add-onboarding
Open

feat: added onboarding#243
null-crafter wants to merge 3 commits into
mainfrom
feat/add-onboarding

Conversation

@null-crafter

@null-crafter null-crafter commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Checklist

  • Tests were added/updated according to the feature/bugfix/change made
  • Version was rolled according to semver requirements
  • API endpoints openapi schema was updated if applicable

Changes

Adds onboarding as provided by https://github.com/iblai/iblai-web-frontend/pull/1871

@null-crafter null-crafter requested a review from mamigot as a code owner June 10, 2026 15:43
@mamigot

mamigot commented Jun 10, 2026

Copy link
Copy Markdown
Member

Claude finished @null-crafter's task —— View job


I'm reviewing this PR for E2E test coverage as the E2E coverage bot.

Todo List

  • Dismiss any stale REQUEST_CHANGES reviews from myself (none found)
  • Identify what changed (git diff) - Found new app/onboarding/page.tsx route
  • Run mechanical coverage check (2 issues found)
  • Read current coverage state (coverage.json and COVERAGE.md)
  • Apply decision logic and post appropriate GitHub review

Coverage Issues Detected

Issue 1: Coverage Regression

  • Checkpoint count decreased from 412 to 403 (−9 checkpoints)
  • This indicates test coverage was removed without replacement

Issue 2: Missing E2E Coverage

  • New route app/onboarding/page.tsx has no E2E journey coverage
  • This is a user-facing route that requires testing

Analysis: This falls under CASE A - New route added but NO new e2e spec and NO coverage update.


@mamigot mamigot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ E2E Coverage Required

This PR adds new routes but does not include e2e journey tests or coverage updates.

Missing coverage for:

  • app/onboarding/page.tsx - New onboarding route needs journey coverage

Additional Issue:

  • Coverage regression detected: checkpoint count decreased from 412 to 403 (−9 checkpoints)
  • Removing test coverage is not allowed without replacement

To fix:

  1. Add a journey spec in e2e/journeys/ testing the onboarding feature
  2. In e2e/coverage.json: add the route to sourceFiles[] and add checkpoint entries with "status": "covered"
  3. In e2e/COVERAGE.md: add matching [x] checkpoint lines
  4. Restore the 9 missing checkpoints or justify their removal

Coverage check output:

Coverage REGRESSION: checkpoints decreased from 412 to 403 (−9)
NEW ROUTES without E2E journey coverage:
  app/onboarding/page.tsx

Run locally to verify:

node e2e/scripts/check-journey-coverage.mjs --all

@mamigot

mamigot commented Jun 11, 2026

Copy link
Copy Markdown
Member

Claude finished @null-crafter's task —— View job


Claude finished @null-crafter's task —— View job


I've completed the E2E coverage review for this PR.

Todo List

  • Dismiss any stale REQUEST_CHANGES reviews from myself (dismissed review #4469401471)
  • Identify what changed (git diff) - Found new app/onboarding/page.tsx route
  • Run mechanical coverage check (2 issues found)
  • Read current coverage state (coverage.json and COVERAGE.md)
  • Apply decision logic and post appropriate GitHub review

Coverage Issues Detected

Issue 1: Coverage Regression

  • Checkpoint count decreased from 419 to 412 (−7 checkpoints)
  • This indicates test coverage was removed without replacement

Issue 2: Missing E2E Coverage

  • New route app/onboarding/page.tsx has no E2E journey coverage
  • This is a user-facing route that requires testing

Analysis: This falls under CASE A - New route added but NO new e2e spec and NO coverage update.

Action Taken: Posted a REQUEST_CHANGES review requiring the developer to add E2E journey tests for the new onboarding route and restore the missing checkpoints.


@mamigot mamigot dismissed their stale review June 11, 2026 17:45

Re-evaluating after new commits.

@mamigot mamigot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ E2E Coverage Required

This PR adds new routes but does not include e2e journey tests or coverage updates.

Missing coverage for:

  • app/onboarding/page.tsx - New onboarding route needs journey coverage

Additional Issue:

  • Coverage regression detected: checkpoint count decreased from 419 to 412 (−7 checkpoints)
  • Removing test coverage is not allowed without replacement

To fix:

  1. Add a journey spec in e2e/journeys/ testing the onboarding feature
  2. In e2e/coverage.json: add the route to sourceFiles[] and add checkpoint entries with "status": "covered"
  3. In e2e/COVERAGE.md: add matching [x] checkpoint lines
  4. Restore the 7 missing checkpoints or justify their removal

Coverage check output:

Coverage REGRESSION: checkpoints decreased from 419 to 412 (−7)
NEW ROUTES without E2E journey coverage:
  app/onboarding/page.tsx

Run locally to verify:

node e2e/scripts/check-journey-coverage.mjs --all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants