Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ The script clones CoGo into `.cache/CodeOnTheGo/` on first run, rebuilds both ja

`local.properties` must contain `sdk.dir=...`. The committed `local.properties` at the repo root is harmless leftover; each plugin needs its own.

## Git workflow

- **Always work on a branch.** Never commit directly to `main` — branch first (`git switch -c ...`) even for a one-line fix. Working on `main` is almost never right for this repo.
- **Fetch before you diff against `main`.** Any time you compute or reason about a diff against `main` (code review, PR base, "what changed"), run `git fetch origin` first and compare against `origin/main`. A stale local `main` produces phantom findings — a `/code-review` here once flagged 3 issues that were outside the actual PR diff because local `main` was ~50 files behind `origin/main`. When a diff-against-main is requested, suggest fetching first.

## Architecture

### `libs/` is the load-bearing piece
Expand Down Expand Up @@ -65,7 +70,15 @@ This is intentional — the `application`-as-library packaging trips those check

### Asset downloads (rare)

Some plugins (currently `ndk-installer-plugin`) register a `downloadAssets` task that fetches a tarball at build time with MD5 verification. `scripts/update-libs.sh` runs it automatically before `assemblePlugin` when the build file references it.
Some plugins (`ndk-installer-plugin`, `ai-literacy-course`) register a `downloadAssets` task that fetches large files at build time with pinned-MD5 verification. These assets are **not committed to git** (e.g. `ai-literacy-course` pulls a ~110 MB course ZIP plus `pdfjs.zip`). `scripts/update-libs.sh` runs `downloadAssets` automatically before `assemblePlugin` when the build file references it.

**Gotcha: a bare `./gradlew assemblePlugin` does NOT run `downloadAssets` and does not warn when the assets are missing** — it silently packages a broken `.cgp` (e.g. a course with no PDF viewer). When building such a plugin by hand, run `./gradlew downloadAssets assemblePlugin` (or the script), and confirm the expected files exist under `src/main/assets/` (or `unzip -l` the `.cgp`) before trusting it.

### One-time on-device install markers

Plugins that extract bundled assets on-device once (currently `ai-literacy-course`, via `CourseInstaller`) gate the work behind a marker file named from a version constant (`INSTALL_VERSION` → `.installed-vN`). If the marker for the current version exists, extraction **and** any post-extraction generation (e.g. `CourseShell.generate()`) are skipped entirely.

**Any change to extraction OR post-extraction generation logic must bump `INSTALL_VERSION`.** Otherwise the change compiles and packages cleanly but has zero effect on existing installs — they keep the stale extracted tree, and it looks like "my fix didn't work" (costing a device round-trip). Bumping the constant forces a clean re-extract. On a device with a prior install, confirm the marker version changed (or wipe plugin data) before concluding a fix works — see Verification below.

## Verification

Expand Down
5 changes: 5 additions & 0 deletions docs/process/learnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ Cross-session gotchas, discoveries, and patterns worth not re-deriving.
- First try `adb -s <device> shell run-as <pkg> cat databases/<file> > local.db`. This only works if the app's APK has `android:debuggable="true"`. Code On The Go's release builds do not, so `run-as` returns "package not debuggable" and writes that error string to your output file.
- On an emulator (root-capable), the fallback is `adb root` (one-time per boot) followed by `adb pull /data/data/<pkg>/databases/<file> .`. This bypasses the debuggable check entirely.

## Plugin build & install gotchas

- **On-device install markers hide code changes.** `ai-literacy-course`'s `CourseInstaller` extracts its bundle once and gates it behind `.installed-v<INSTALL_VERSION>`; if the marker exists, extraction *and* `CourseShell.generate()` are skipped. A logic fix (e.g. lesson-item ordering) has zero on-device effect until `INSTALL_VERSION` is bumped — it looks like "the fix didn't work" and costs a device round-trip. Bump the version constant as part of any extraction/generation change.
- **`assemblePlugin` silently ships broken `.cgp`s when downloaded assets are missing.** Plugins with a `downloadAssets` task (`ai-literacy-course` → course ZIP + `pdfjs.zip`; `ndk-installer-plugin`) don't fetch those assets during a plain `assemblePlugin`, and there's no build-time warning — the missing asset only surfaces as a runtime failure on device (`Bundled asset not found: pdfjs.zip` → "Could not prepare the course"). Run `./gradlew downloadAssets assemblePlugin` (or `scripts/update-libs.sh`) and `unzip -l` the `.cgp` to confirm assets are present before handing it over.

## PDF debugging without pdftotext

- When `pdftotext`/`mutool`/`qpdf` aren't installed and you need to read a PDF's /Title or /Author, the Info dictionary is often inside a flate-encoded object stream. Walk every `stream ... endstream` block, `zlib.decompress` each, and regex for `/(Title|Author|Subject|Creator|Producer|Keywords)\s*\(...\)` in the decoded bytes — and for hex-encoded `<FEFF...>` UTF-16BE strings. This recovered the Morelli & Walde authorship of `JavaJavaJava.pdf` in the bookshelf session.
39 changes: 39 additions & 0 deletions docs/process/retrospective.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,44 @@
# Retrospectives

## 2026-07-01 — Code review (PR #31) + AI Literacy Course ordering fix (PR #36)

### Time Breakdown
| Started (UTC) | Phase | 👤 Hands-On | 🤖 Agent | Problems |
|---|---|---|---|---|
| 00:07 | Code review (8-angle fan-out, verify, ≤10 findings) | █ 7m | █████ 47m | ⚠ scope > PR (stale local `main`) |
| 00:54 | Post findings 1–10 as PR comments | ▌ 3m | ▌ 5m | ⚠ 3 findings outside PR diff → top-level fallback |
| 00:59 | Housekeeping (main, pull, ls) | ▌ 1m | ▌ 1m | |
| 01:00 | Ordering fix — plan mode + implement | █ 6m | ██ 20m | ⚠ plan rejected → "just branch & do it" |
| 01:20 | Device-test loop (build → fix → rebuild) | ██ 15m | ███ 28m | ⚠ 2 rework cycles: install-marker gate, missing `pdfjs.zip` |
| 01:48 | Commit / push / PR + away | ▌ 1m | ▌ 3m | idle ~38m before retro |

### Metrics
| Metric | Duration |
|---|---|
| Total wall-clock | ~2h 19m |
| Hands-on (script) | 48m (35%) — inflated by `/code-review` + `/retro` command expansions counted as typing; real ≈ 25–30m |
| Automated agent time | ~51m (37%) |
| Idle / device-testing / away | ~40m (29%) |
| Retro analysis time | ~2 min |

### Key Observations
- **Code review ran against a stale local `main`** (150 files vs PR #31's 100); 3 compose-preview findings fell outside the PR diff and needed a fallback top-level comment. No `git fetch` happened first.
- **Two avoidable device round-trips** on the ordering fix: (1) the `.installed-vN` marker gate meant the fix had no effect until `INSTALL_VERSION` was bumped — I warned about it but didn't preempt it; (2) `pdfjs.zip` was never downloaded on this machine and `assemblePlugin` silently shipped a viewer-less `.cgp`.
- **Plan mode was overhead** for a ~15-line fix; user rejected the plan with "make a branch; do the work there."
- **Worked well:** the review fan-out (6 parallel finder agents, self-verified) ran with near-zero interaction; the ordering bug was root-caused on the first try and validated via a Python port of the sort (no `kotlinc` locally).

### Feedback
**What worked:** (not separately volunteered — core review + fix landed cleanly).
**What didn't:** Always work on a branch — working on `main` is rarely right. When diffing against `main`, suggest fetching recent changes to avoid stale situations. The `INSTALL_VERSION` miss was sloppy and wasted a cycle.

### Actions Taken
| Issue | Action Type | Change |
|---|---|---|
| Worked on `main`; stale-`main` diff produced phantom findings | CLAUDE.md | Added "Git workflow" section: always branch; `git fetch` + compare `origin/main` before any diff-against-main |
| `INSTALL_VERSION` not bumped with generation-logic change → dead fix on device | CLAUDE.md | Added "One-time on-device install markers" subsection: bump `INSTALL_VERSION` on any extraction/generation change |
| `assemblePlugin` silently shipped a `.cgp` missing `pdfjs.zip` | CLAUDE.md | Expanded "Asset downloads": warn assemblePlugin skips `downloadAssets` and ships broken `.cgp`; verify assets first |
| Same two build traps, cross-session | Learnings | Added "Plugin build & install gotchas" section to `docs/process/learnings.md` |

## 2026-06-08 — Importing the bookshelf plugin from contrib

### Time Breakdown
Expand Down