Updating Production S3 with latest icog-books

[amitsinghsutara]

Summary by CodeRabbit

• New Features
  • Added support for a separate partner content deployment flow.
• Bug Fixes
  • Improved app startup and loading behavior for more reliable content access.
  • Added clearer completion messages after content syncing.
• Chores
  • Updated automated review and release workflow settings for stricter checks and notifications.

[coderabbitai]

Warning

Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories.

📝 Walkthrough

Walkthrough

CircleCI S3 sync jobs gain explicit bucket targets, completion messages, and a new partner-sync job with branch-based routing. The CodeRabbit configuration is rewritten to a newer schema covering reviews, path filters, pre-merge checks, and chat. App.ts exports several previously private helpers, activates Firebase session logging, bumps the app version, and replaces direct App instantiation with a BookLoader factory selecting GdlBookLoader or CrBookLoader.

Changes

CircleCI S3 Sync Updates

Layer / File(s)	Summary
S3 sync jobs and partner job .circleci/config.yml	Development and production sync jobs target explicit buckets and print completion messages; a new s3-sync-partner job syncs to a partner development bucket.
Workflow branch routing .circleci/config.yml	Workflow filters route s3-sync-partner to the Partner branch and restrict s3-sync-production to main.

CodeRabbit Configuration Rewrite

Layer / File(s)	Summary
Schema header and review workflow settings .coderabbit.yaml	Adds schema directive/language settings and replaces require_approval/labels.enabled with request_changes_workflow, fail_commit_status, suggested_labels, and auto_apply_labels.
Auto-review scoping and path filters .coderabbit.yaml	Replaces branch include/exclude and ignore patterns with base_branches and path_filters excluding docs/dist and markdown/JSON files.
Pre-merge checks and chat settings .coderabbit.yaml	Replaces the checks block with custom_checks covering tests, lint, and 70% coverage plus title/description rules, and enables chat.auto_reply while removing the notifications block.

App.ts Loader Refactor and Firebase Session Logging

Layer / File(s)	Summary
Module setup, version bump, and Firebase logging App.ts	Bumps appVersion, initializes sessionStartTime at module load, exports firebaseAnalyticsManager, and activates Firebase session-start payload logging in the App constructor.
Exported helper functions App.ts	Marks handleLoadingMessage, logDownloadProgressWithPayloadToFirebase, readLanguageDataFromCacheAndNotifyAndroidApp, and handleUpdateFoundMessage as exported, adding a sessionStartTime fallback guard.
BookLoader factory and startup flow App.ts	Adds createBookLoader to select GdlBookLoader or CrBookLoader by bookName prefix and replaces App instantiation with loader.load() at startup.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AppTs
  participant createBookLoader
  participant GdlBookLoader
  participant CrBookLoader
  AppTs->>createBookLoader: bookName
  createBookLoader->>GdlBookLoader: return if prefix gdl-
  createBookLoader->>CrBookLoader: return otherwise
  AppTs->>GdlBookLoader: load()
  AppTs->>CrBookLoader: load()

Loading

Possibly related PRs

• curiouslearning/CRWebPlayer#211: Both PRs modify `.coderabbit.yaml` to change CodeRabbit review/gating configuration.
• curiouslearning/CRWebPlayer#234: Overlaps in `App.ts` version bump/exported helpers and `.coderabbit.yaml` workflow config edits.
• curiouslearning/CRWebPlayer#288: Overlaps in adding the s3-sync-partner job with the same partner S3 destination and branch-filtered routing.

Suggested reviewers: `dz4va`

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Tests & Lint & Coverage	⚠️ Warning	No tests/spec files were added or updated; package.json only has build/dev/injectManifest scripts, and there’s no lint/coverage tooling/config to verify.	Add/update tests for the changed code, define and run lint and coverage commands, and ensure coverage tooling reports at least 70%.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and describes the main S3 content update, including the production bucket refresh.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch develop

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (4)

.circleci/config.yml (2)

60-85: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

New s3-sync-partner job duplicates outdated Python image.

The new job repeats cimg/python:3.6, which is end-of-life (Python 3.6 support ended in Dec 2021). Since this build environment isn't actually running Python code (just AWS CLI sync via the orb), this is mostly a supply-chain/security hygiene concern (unpatched base image) rather than a functional one, but worth bumping alongside the other two jobs while you're touching this file.

♻️ Suggested image bump

   s3-sync-partner:
     docker:
-      - image: "cimg/python:3.6"
+      - image: "cimg/python:3.11"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.circleci/config.yml around lines 60 - 85, The new s3-sync-partner job is
using the outdated cimg/python:3.6 base image, so update the docker image in the
s3-sync-partner job to match the newer image used by the other sync jobs. Keep
the aws-s3/sync step and its settings unchanged; only replace the obsolete
Python 3.6 runtime reference in the job definition.

---

6-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Significant duplication across the three sync jobs.

s3-sync-development, s3-sync-production, and the new s3-sync-partner are near-identical copies differing only in bucket URL, region variable, and echo text. Consider a parameterized CircleCI job (or YAML anchors) to avoid triple-maintaining the same steps/args when exclusions or ACL settings need to change in the future.

♻️ Example parameterized job sketch

+jobs:
+  s3-sync:
+    parameters:
+      bucket:
+        type: string
+      region_env:
+        type: env_var_name
+        default: AWS_DEFAULT_REGION
+    docker:
+      - image: "cimg/python:3.11"
+    steps:
+      - checkout
+      - run:
+          name: Pre-sync Setup
+          command: echo "Setting up for S3 sync to <<parameters.bucket>>"
+      - aws-s3/sync:
+          arguments: |
+            --acl public-read \
+            --cache-control "max-age=86400" \
+            --exclude ".git/*" \
+            --exclude ".gitignore" \
+            --exclude ".gitattributes"
+          aws-access-key-id: AWS_ACCESS_KEY
+          aws-region: <<parameters.region_env>>
+          aws-secret-access-key: AWS_SECRET_ACCESS_KEY
+          from: ./
+          to: "s3://<<parameters.bucket>>"
+      - run:
+          name: Post-sync Confirmation
+          command: echo "Sync to <<parameters.bucket>> S3 bucket completed."

Then invoke in the workflow with different bucket/region_env params per branch.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.circleci/config.yml around lines 6 - 85, The three CircleCI jobs
s3-sync-development, s3-sync-production, and s3-sync-partner duplicate the same
checkout/run/aws-s3/sync steps with only the bucket, region, and log text
changing. Refactor this into a single parameterized job or reusable YAML anchor
in .circleci/config.yml, and pass bucket/region-specific values from the
workflow so future changes to aws-s3/sync arguments or exclusions only need to
be made once.

.coderabbit.yaml (2)

30-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Excluding all **/*.json drops review coverage on meaningful config.

The !**/*.json filter suppresses review of files like package.json, tsconfig.json, and Firebase/app config JSON — where dependency bumps or config changes can carry real risk. Consider excluding only large/generated JSON (e.g. package-lock.json, which you already list) rather than every JSON file.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.coderabbit.yaml around lines 30 - 35, The path filter list is excluding
every JSON file, which prevents review of important config like package.json,
tsconfig.json, and app/firebase JSON. Update the path_filters in the
.coderabbit.yaml config to stop blanket-excluding **/*.json and instead only
ignore generated or large JSON files that should not be reviewed, keeping
meaningful config files visible to code review.

---

22-27: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Anchor base_branches for exact branch matches. base_branches entries are regexes, so test, develop, and main can match more than the intended branch names. Use ^...$ if these are meant to be exact.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.coderabbit.yaml around lines 22 - 27, The base_branches patterns are
currently treated as regexes, so the plain entries in the base_branches list can
match unintended branch names. Update the base_branches values in the
configuration to use exact-match regex anchors for develop, test, and main, and
keep the logic consistent with how the review selection matches branches.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@App.ts`:
- Around line 241-243: The startup flow in App.ts is ignoring the Promise
returned by loader.load(), so any loader rejection can become an unhandled
promise rejection. Update the App.initialize path around createBookLoader and
loader.load() to either await the call inside the existing async flow or attach
a .catch() that forwards the error into the same recovery/error handling used by
App.initialize, so failures from initializeGdlBook and other loaders are handled
consistently.

---

Nitpick comments:
In @.circleci/config.yml:
- Around line 60-85: The new s3-sync-partner job is using the outdated
cimg/python:3.6 base image, so update the docker image in the s3-sync-partner
job to match the newer image used by the other sync jobs. Keep the aws-s3/sync
step and its settings unchanged; only replace the obsolete Python 3.6 runtime
reference in the job definition.
- Around line 6-85: The three CircleCI jobs s3-sync-development,
s3-sync-production, and s3-sync-partner duplicate the same
checkout/run/aws-s3/sync steps with only the bucket, region, and log text
changing. Refactor this into a single parameterized job or reusable YAML anchor
in .circleci/config.yml, and pass bucket/region-specific values from the
workflow so future changes to aws-s3/sync arguments or exclusions only need to
be made once.

In @.coderabbit.yaml:
- Around line 30-35: The path filter list is excluding every JSON file, which
prevents review of important config like package.json, tsconfig.json, and
app/firebase JSON. Update the path_filters in the .coderabbit.yaml config to
stop blanket-excluding **/*.json and instead only ignore generated or large JSON
files that should not be reviewed, keeping meaningful config files visible to
code review.
- Around line 22-27: The base_branches patterns are currently treated as
regexes, so the plain entries in the base_branches list can match unintended
branch names. Update the base_branches values in the configuration to use
exact-match regex anchors for develop, test, and main, and keep the logic
consistent with how the review selection matches branches.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Handle rejection from the async loader.load().

load() returns a Promise<void>, but the returned promise is neither awaited nor given a .catch(). Any rejection escaping the loaders (e.g., before App.initialize's internal try/catch, or from initializeGdlBook) surfaces as an unhandled promise rejection with no user-facing recovery on the startup path.

🛡️ Proposed fix

 // Create and use the appropriate loader based on the book type
 const loader: BookLoader = createBookLoader(bookName);
-loader.load();
+loader.load().catch((error) => {
+  console.error("Failed to load book:", error);
+});

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Create and use the appropriate loader based on the book type
	const loader: BookLoader = createBookLoader(bookName);
	loader.load();
	// Create and use the appropriate loader based on the book type
	const loader: BookLoader = createBookLoader(bookName);
	loader.load().catch((error) => {
	console.error("Failed to load book:", error);
	});

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@App.ts` around lines 241 - 243, The startup flow in App.ts is ignoring the
Promise returned by loader.load(), so any loader rejection can become an
unhandled promise rejection. Update the App.initialize path around
createBookLoader and loader.load() to either await the call inside the existing
async flow or attach a .catch() that forwards the error into the same
recovery/error handling used by App.initialize, so failures from
initializeGdlBook and other loaders are handled consistently.