Migrate from python-for-android to Chaquopy#259
Conversation
rtibbles
force-pushed
the
chaquopy_refactor
branch
5 times, most recently
from
9007d2d to
4eb9820
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Thanks for this major migration! Moving from python-for-android to Chaquopy is a great step forward - the reduced APK size (~200MB to ~74MB) and faster build times (25min to 2min) are impressive wins. The new WebView-based architecture is clean and the code is well-organized.
I found no blocking issues - CI is green and the manual verification screenshots look solid. I have several suggestions and nitpicks that could improve robustness, particularly around edge-case error handling and thread safety.
Summary of findings:
- 0 blocking issues
- 8 suggestions (resource safety, null guards, thread safety, design clarity)
- 3 nitpicks (logging, unused code, minor style)
Nice work on the migration! The architecture is significantly cleaner than the p4a bootstrap approach.
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Thanks for this ambitious migration from python-for-android to Chaquopy! This is clearly a significant improvement -- the APK size reduction, build time improvement, and architectural cleanup are all excellent outcomes. The PR is well-structured with a thorough description and manual verification steps.
I found several issues that should be addressed before merging, organized by severity:
Blocking issues (5):
- CI failure:
patchKolibriTartask fails when no tar file is present, breaking unit tests - WebView security:
MIXED_CONTENT_ALWAYS_ALLOWis unnecessarily permissive - Thread safety:
KolibriServerService.startHttpServer()has a race window for duplicate thread creation - Supply chain risk:
fkirc/skip-duplicate-actions@masteris unpinned in CI - Task reconciler: Tag filtering heuristic for distinguishing job IDs from class names is fragile
Key suggestions (not blocking but strongly recommended):
- WebView
setAllowFileAccess(true)and third-party cookies are unnecessary security relaxations KolibriEnvironmentSetup.initializedshould bevolatilefor thread-safe reads- Server state should be reset in
KolibriServerService.onDestroy() - Duplicated tar extraction logic between configuration-phase and task should be consolidated
The inline comments below have the full details. Looking forward to the updates!
rtibbles
force-pushed
the
chaquopy_refactor
branch
4 times, most recently
from
4d6a145 to
359de57
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Impressive migration from python-for-android to Chaquopy. The APK size reduction, build time improvement, and move to standard Android project structure are significant wins.
CI passing (all 7 checks green). Screenshots confirm the Learn page and channel navigation work correctly.
Blocking (not addressable as inline comments):
app/src/main/AndroidManifest.xml:92—android:permission="android.permission.FOREGROUND_SERVICE_CONNECTED_DEVICE, android.permission.FOREGROUND_SERVICE_DATA_SYNC"is invalid.android:permissionaccepts only a single permission string. This comma-separated value is treated as one literal string, which will never match. Either remove this attribute (theforegroundServiceTypedeclaration +<uses-permission>should suffice) or split into separate service declarations..github/workflows/build_apk.yml:190—cat .version-codereads a file that the new Chaquopy-based build never generates. This step will fail or produce empty output, breaking the release pipeline. Either write this file during the build or extract the version code from the APK viaaapt2 dump badging.app/src/main/AndroidManifest.xml:17— Stale comment says<!-- Allow writing to external storage -->but the permission on line 18 isandroid.permission.INTERNET.
See inline comments for remaining blocking issues and suggestions.
|
Thanks for the review, taking a look now. |
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Significant progress since last review — the vast majority of prior findings have been addressed.
CI passing (all 7 checks green: linting, build, unit tests, smoke test). PR targets develop (the default branch).
21 prior finding(s) resolved. The WebView security hardening, thread safety fixes, supply chain pinning, task reconciler prefix tags, try-with-resources, volatile fields, and other feedback have all been incorporated cleanly.
Unresolved prior findings:
-
[suggestion]
AndroidManifest.xml:92— comma-separatedandroid:permissionis invalid — UNRESOLVED.android:permissionaccepts a single permission string, but line 92 has"android.permission.FOREGROUND_SERVICE_CONNECTED_DEVICE, android.permission.FOREGROUND_SERVICE_DATA_SYNC"which is one literal string containing a comma. This doesn't break the app (the attribute only restricts external callers, and this is an internal AndroidX service), but it's misleading — either remove the attribute entirely (theforegroundServiceType+<uses-permission>declarations are sufficient) or split into separate<service>declarations. -
[suggestion]
.github/workflows/build_apk.yml:190—cat .version-codereads a file that the new build never generates — UNRESOLVED. The Chaquopy/Gradle build computesversionCodeat configuration time inapp/build.gradlebut never writes a.version-codefile. This step currently produces an emptyversion-codeoutput. It doesn't break CI (the output isn't consumed bybuild_and_test.yml), but it will break the release pipeline whenrelease_apk.ymltries to use this value. Either write the file during the build (e.g., adoLastin the Gradle build) or extract the version code from the APK viaaapt2 dump badging. -
[nitpick]
AndroidManifest.xml:17— stale comment — UNRESOLVED. Comment says<!-- Allow writing to external storage -->but the permission on line 18 isandroid.permission.INTERNET.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
Revision SummaryBlocking items fixed
Suggestions addressed
Declined (with reasoning)
|
|
Addressed the three items from the review body in
|
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior findings addressed. The latest commit (0b8e49e) fixes the invalid comma-separated android:permission, removes the stale manifest comment, and writes .version-code for CI.
CI: linting, build, unit tests all passing. Smoke test in progress.
25+ prior finding(s) resolved.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior findings from previous reviews have been resolved — nice work addressing everything systematically.
CI passing (Linting green). One new suggestion below.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior blocking issues, suggestions, and nitpicks from previous reviews have been addressed. Nice work on the thorough revisions.
23 prior finding(s) resolved.
CI passing (Linting green). Two new minor findings below.
- [suggestion] Lambda parameter shadows field in
WorkController.dispatch()— see inline comment - [nitpick]
WorkControllerService.onDestroy()missingsuper.onDestroy()— see inline comment
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior findings from three rounds of review have been thoroughly addressed. The fixes are solid — highlights include the synchronized startHttpServer() eliminating the race window, MIXED_CONTENT_NEVER_ALLOW + disabled file/content access hardening the WebView, the kolibri:job: tag prefix making reconciliation robust, scoped network security config replacing blanket cleartext, and the CompletableFuture-based async init preventing ANR.
CI passing (7/7 checks green). Code is clean and well-structured.
16 prior finding(s) resolved.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibbles
force-pushed
the
chaquopy_refactor
branch
2 times, most recently
from
ed75ba4 to
382f479
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior blocking issues, suggestions, and nitpicks from the three previous reviews have been addressed. The WebView security hardening (MIXED_CONTENT_NEVER_ALLOW, file access disabled, scoped network security config, custom WebViewClient), thread safety fixes (synchronized startHttpServer, volatile fields, CompletableFuture-based async init), CI fixes (pinned actions, .version-code generation), manifest corrections, and task reconciler improvements are all solid.
CI: Linting, build, and unit tests passing. Smoke test pending.
~25 prior finding(s) resolved.
Prior findings still open:
- [suggestion] Unquoted
${{ }}in CI workflowrun:steps — PARTIALLY_ADDRESSED.build_and_test.ymlwas properly quoted (tar="${{ ... }}"), butbuild_apk.ymllines 134 and 137 still use unquoted${{ github.event.inputs.tar-url }}/${{ inputs.tar-url }}. Low risk sinceworkflow_dispatchrequires write access, but quoting or using an env var is best practice per GitHub's security hardening guide.
New findings:
- See inline comment on
onTrimMemory.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibbles
force-pushed
the
chaquopy_refactor
branch
from
382f479 to
1009374
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior findings have been thoroughly addressed across the revision commits. Nice work on the fixes.
16 prior finding(s) resolved. All blocking issues (manifest permissions, CI .version-code, WebView security, thread safety, supply chain pinning, task reconciler heuristic) and suggestions (volatile fields, server state reset, cleartext scoping, field shadowing, external URL handling, async init) are confirmed fixed in the current code.
CI: 6/7 checks green (lint, build, unit tests, download tar, path checks). Smoke test still in progress — worth confirming before merge.
One new nitpick below.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibbles
force-pushed
the
chaquopy_refactor
branch
2 times, most recently
from
2bc1141 to
b891f16
Compare
rtibbles
force-pushed
the
chaquopy_refactor
branch
from
830ee20 to
86be385
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Solid migration from python-for-android to Chaquopy. The codebase is significantly cleaner — well-structured Java/Python separation, proper Android lifecycle management, and the WebView architecture is sound.
CI: 6/7 checks passing, smoke test pending. All prior blocking findings have been addressed.
All prior findings resolved.
New findings:
- 1 suggestion (see inline comment)
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibbles
force-pushed
the
chaquopy_refactor
branch
2 times, most recently
from
c018a7a to
a8c52e4
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Thorough delta review of the Chaquopy migration. All prior findings from three rounds of review have been addressed — impressive follow-through.
CI passing. Screenshots verify the Learn page and channel navigation render correctly.
29 prior finding(s) resolved. 1 prior finding acknowledged (foreground service omitted by design to avoid battery drain — reasonable trade-off).
New findings:
- 1 suggestion (inline):
WorkController.mInstancemissingvolatilein double-checked locking pattern
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibbles
force-pushed
the
chaquopy_refactor
branch
from
a8c52e4 to
6e4e9e9
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Major migration from p4a to Chaquopy — all prior blocking findings have been addressed. The WebView security hardening, kolibri:job: tag prefix, CI pinning, and manifest fixes are all solid.
CI passing. Screenshots verified.
19 prior finding(s) resolved. 1 prior suggestion acknowledged (foreground service design — deliberate choice per author's reasoning).
New findings:
- 0 blocking
- 2 suggestions (WebView null safety, CI expression interpolation)
- 1 nitpick (CI release lookup robustness)
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibbles
force-pushed
the
chaquopy_refactor
branch
from
6e4e9e9 to
c868d53
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior findings from Reviews 2 and 3 have been addressed — nice work on the thorough revisions.
14 prior finding(s) resolved. 2 findings acknowledged (foreground service design, WebViewClient approach — both reasonable).
CI passing. No blocking issues remain. A couple of minor suggestions below.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Strong migration — all prior findings have been addressed. CI passing.
All prior findings resolved. The author systematically addressed every blocking issue, suggestion, and nitpick from three previous reviews, including the manifest permission fix, .version-code generation, network security scoping, WebView hardening, thread safety improvements, and async initialization.
New finding (1 blocking):
Application.getProcessName()crashes on API 24–27 (see inline comment)
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibbles
force-pushed
the
chaquopy_refactor
branch
from
c868d53 to
a116a2c
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
All prior issues addressed. Clean migration from p4a to Chaquopy with solid architecture.
CI passing. Screenshots verified — Learn page and channel navigation render correctly.
24 prior finding(s) resolved. 1 prior finding acknowledged (foreground service design — deliberate choice to avoid battery drain).
No new blocking issues found. The code is well-structured: WebView security is properly locked down (MIXED_CONTENT_NEVER_ALLOW, file access disabled, scoped cleartext config), thread safety is handled correctly (synchronized server start, volatile fields, CompletableFuture chains), and the task reconciliation system uses explicit kolibri:job: tag prefixes instead of heuristics. The CI dependency is pinned (skip-duplicate-actions@v5.3.1) and .version-code is now generated during the build.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
rtibbles
force-pushed
the
chaquopy_refactor
branch
from
a116a2c to
fc1ddce
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Thorough migration from p4a to Chaquopy. All prior findings resolved.
Prior findings
All 19 prior findings remain resolved — no regressions since last review.
CI failing: ruff-format rejects kolibri_plugin.py:47. 1 blocking finding below.
Findings:
- 1 blocking (CI lint failure)
- 1 praise
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
| class AndroidShareFileHook(ShareFileHook): | ||
| def share_file(self, *args, **kwargs): | ||
| return share_by_intent(*args, **kwargs) | ||
| def share_file(self, path=None, filename=None, message=None, app=None, mimetype=None): |
There was a problem hiding this comment.
blocking: CI ruff-format check is failing because this line exceeds the formatter's line-length threshold. The fix is straightforward — run ruff format locally or split the signature and body across multiple lines:
def share_file(
self, path=None, filename=None, message=None, app=None, mimetype=None
):
ShareUtils.shareByIntent(
path or "", message or "", app or "", mimetype or ""
)This is the only file causing the CI failure.
| } | ||
| } | ||
|
|
||
| private boolean isMainProcess() { |
There was a problem hiding this comment.
praise: Good handling of the isMainProcess() API level split — using Application.getProcessName() on API 28+ and falling back to ActivityManager.getRunningAppProcesses() on older devices. This avoids the crash that the earlier Application.getProcessName() approach would have had on API 24-27.
rtibbles
force-pushed
the
chaquopy_refactor
branch
from
fc1ddce to
e6c6500
Compare
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Clean migration from python-for-android to Chaquopy. All prior findings addressed.
Prior findings
All prior findings remain resolved or acknowledged — no changes since last review.
CI passing (Linting green). Screenshots verified in PR description — Learn page and channel navigation render correctly.
Findings:
- 0 blocking
- 0 suggestions
- 2 praise (see inline comments)
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Compared the current PR state against findings from a prior review:
- Retrieved prior bot reviews deterministically via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
- Checked CI status and linked issue acceptance criteria
|
|
||
| // No file:// or content:// access needed — Kolibri loads via http://127.0.0.1 | ||
| settings.setAllowFileAccess(false); | ||
| settings.setAllowContentAccess(false); |
There was a problem hiding this comment.
praise: Thorough WebView security posture — MIXED_CONTENT_NEVER_ALLOW, file/content access disabled, third-party cookies rejected, scoped cleartext via network security config, and external URLs delegated to the system browser. This is a textbook lockdown for a localhost-only WebView.
| if (initFuture != null) { | ||
| return; | ||
| } | ||
| initFuture = CompletableFuture.runAsync(() -> initializeEnv(context, skipUpdate)); |
There was a problem hiding this comment.
praise: The CompletableFuture-based async init is well-designed — App.onCreate() kicks it off without blocking (avoiding ANR), the server thread and work controller both awaitInit() before proceeding, and the synchronized + initFuture != null guard ensures exactly-once execution across processes. Clean separation of the "when to init" decision from the "what to init" logic.
|
|
||
| // Enable required plugins BEFORE initialize() | ||
| // Plugins must be enabled before the registry is initialized | ||
| kolibriMain.callAttr("enable_plugin", "kolibri.plugins.app"); |
There was a problem hiding this comment.
This needs to be removed now.
|
Notes from code walkthrough with @bjester and @ozer550:
|
rtibbles
force-pushed
the
chaquopy_refactor
branch
4 times, most recently
from
6cc459e to
c78bddd
Compare
Move reusable source files to new app/ layout via git mv. Delete kivy/renpy runtime, vendored jtar/jnius libraries, Docker build, Buildkite CI, old SQLite layer, and Sentinel/Reconciler/StateMap task infrastructure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the python-for-android/kivy build system with a standard Gradle project using Chaquopy for Python integration. This is the core of the architecture migration. Build system: - Gradle wrapper (8.11.1), root and app build files - Chaquopy plugin for embedding Python in the Android app - Updated .gitignore, requirements.txt, pyproject.toml Java application layer: - App, WebViewActivity, KolibriWebChromeClient for WebView-based UI - KolibriServerService/ViewModel for managing the Kolibri HTTP server - KolibriEnvironmentSetup for Python environment configuration - WorkController/WorkControllerService for background task orchestration - Worker classes (BaseTaskWorker, ForegroundWorker, BackgroundWorker) - Task management (Task, TaskWorkerImpl, WorkerImpl, Observable/Observer) - Notification system (Builder, Manager, NotificationRef, Notifier) - Utility classes (AuthUtils, ContextUtil, NetworkUtils, ShareUtils) Python application layer: - main.py entry point for Kolibri server lifecycle - taskworker.py and task_reconciler.py for background task execution - auth.py for authentication token generation - Kolibri plugin and settings modules - monkey_patch_zeroconf.py for Android compatibility Android resources: - WebView layout with animated splash screen - Kolibri logo vector drawable with wing-flap animation - Notification icons, themes, colors - Network security config, file provider paths - Localized HTML content for loading/error screens Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add build_and_test.yml for running unit tests in CI - Update build_apk.yml and release_apk.yml for Gradle-based builds - Remove pr_build.yml (replaced by build_and_test.yml) - Update pre-commit.yml and .pre-commit-config.yaml (add ruff, spotless) - Overhaul Makefile for Gradle/Chaquopy workflow (SDK setup, emulator, build, install, test targets) - Simplify scripts/version.py, create_strings.py, play_store_api.py for the new project structure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- AuthUtilsTest: token generation, URL building, header extraction - ContextUtilTest: singleton initialization and context access - ShareUtilsTest: share intent construction - BaseTaskWorkerTest: Python task delegation and error handling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rewrite documentation to reflect the new build system, project structure, and development workflow. Covers Gradle setup, Chaquopy Python integration, emulator usage, and CI configuration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- CLAUDE.md and AGENTS.md for AI-assisted development workflow - CDP helper (scripts/cdp_helper.py) for WebView DOM inspection and interaction via Chrome DevTools Protocol over ADB - Unit tests for CDP helper - Maestro smoke test for app launch verification Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the apt dependencies step (buildozer-era packages not needed by Chaquopy) and the make setup step (SDK, NDK, build-tools, platform-tools are all preinstalled on ubuntu-latest). The runner provides ANDROID_SDK_ROOT which the Makefile already respects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rtibbles
force-pushed
the
chaquopy_refactor
branch
from
c78bddd to
e37bade
Compare
3 participants
Summary
Migrates the Kolibri Android app from python-for-android (p4a) to Chaquopy for Python integration. This is a major architectural change that:
python-for-android/dists/kolibri/to standardapp/moduleCommit history
Manual verification performed
./gradlew assembleDebug./gradlew test) — all passScreenshots
References
Reviewer guidance
How to test
Follow the Development Setup section in the updated README:
make setupto download Android SDK, system image, and create the emulator AVDpython3 -m venv venv && source venv/bin/activate && pip install -r build-requirements.txttar/directorymake emulatorto start the emulatormake installto build and install the APKCI validation
The
build_apkworkflow should produce a valid APK artifact — please verify the CI check completes successfully. Thebuild_and_testworkflow runs unit tests on every push.Risky areas requiring careful review
Key architectural changes
🤖 Generated with Claude Code