Independent read-only review of 8a47d30/bbbff77/16c46a5 per handoff/claude-master-handoff.md's review-and-freeze step. Verified build/test/lint independently rather than trusting recorded evidence. Fixed P2: requireSuccess() collapsed HTTP 409/422 into the generic InvalidResponse, which would have blocked TASK-007's planned conflict/unprocessable error states. Added ApiFailure.Conflict and ApiFailure.Unprocessable with test coverage. Recorded a P3 (CPU-bound work on the Main dispatcher in CodeBrowserViewModel) for TASK-016 rather than fixing it now. Closed TASK-005/006's mutual contract-freeze checkboxes. No P0/P1 found. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
54 lines
5.2 KiB
Markdown
54 lines
5.2 KiB
Markdown
# TASK-006: Browse trees and source code by ref
|
|
|
|
Status: implemented and independently reviewed by Claude; no P0/P1 findings; device check deferred
|
|
Owner: Codex
|
|
Reviewer: Claude
|
|
|
|
## Outcome
|
|
|
|
A user can navigate directories and view source files at an explicit branch, tag, or commit without silently changing refs.
|
|
|
|
## Writable paths
|
|
|
|
- `app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/`
|
|
- `app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/`
|
|
- `app/src/main/res/values/strings_codebrowser.xml`
|
|
- TASK-006 evidence only
|
|
- `app/src/main/java/com/ubisam/ubgitea/MainActivity.kt` (feature wiring only)
|
|
- `app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListScreen.kt` (selected-ref handoff only)
|
|
- `app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt` (selected-ref handoff only)
|
|
|
|
Shared domain, data, Gradle, and manifest contracts remain unchanged. Codex temporarily owns implementation because the product owner paused Claude work while retaining Claude as the future independent reviewer.
|
|
|
|
## Acceptance criteria
|
|
|
|
- [x] Repository, selected ref, and current path remain visible.
|
|
- [x] Directory navigation is deterministic and safely handles encoded names.
|
|
- [x] Text files render with bounded memory use and copy support.
|
|
- [x] Binary, Git LFS, submodule, unsupported encoding, and oversized-file states have explicit fallbacks.
|
|
- [x] Loading, empty, offline, authentication-expired, not-found, and server-error states are localized and accessible.
|
|
- [x] Tests cover ref preservation, path navigation, content classification, and size limits.
|
|
- [x] Resource names use the `codebrowser_` prefix.
|
|
- [x] Evidence records the exact reviewed TASK-005 contract-freeze commit.
|
|
|
|
## Evidence
|
|
|
|
- Implemented against TASK-005 local completion commit `bbbff77`. It remains a provisional integration base until the deferred independent review closes the contract-freeze criterion.
|
|
- Selecting a branch or tag opens its root tree. Every load carries immutable repository coordinates and an explicit ref; no navigation action substitutes the default branch.
|
|
- Breadcrumb and parent navigation operate on validated `RepositoryPath` values without decoding encoded-looking names. Directory rows use the server-returned validated path, and directory results are bounded, deduplicated, directory-first, and deterministic.
|
|
- Source viewing reuses the reviewed TASK-006A strict classifier with 1 MB input, 200,000-code-point, and 10,000-line display limits. Text is selectable and copyable; truncation, binary, LFS, unsupported UTF-8, oversized, symlink, and submodule states are explicit.
|
|
- Focused code-browser tests and repository-wide `assembleDebug testDebugUnitTest lintDebug` passed: 102 unit tests across 23 suites, zero failures. `git diff --check` and focused sensitive-pattern/read-only scans passed.
|
|
- Device interaction and independent review remain deferred while Codex owns the serial implementation stream.
|
|
|
|
## Claude independent review closure
|
|
|
|
Reviewed read-only at commit `16c46a5` per `handoff/claude-master-handoff.md`. Independently reran `assembleDebug testDebugUnitTest lintDebug --rerun-tasks`: BUILD SUCCESSFUL, 102 tests / 23 suites, 0 failures; `git diff --check` clean.
|
|
|
|
- Root back (verified, no finding): `navigateUp()` returns `false` at the root path instead of calling `loadDirectory`; `MainActivity` treats a `false` result from `onBack` as "leave the code browser" (`codeBrowserViewModel.close(); browsingCode = false`), and both `CodeBrowserScreen` and `RepositoryListScreen` register `BackHandler(onBack = ...)`, so the hardware/gesture back button reaches this logic rather than exiting the Activity directly.
|
|
- Rotation/state restoration (partially verified, tracked not new): the `browsingCode` screen-mode flag survives configuration change via `rememberSaveable`, and `CodeBrowserViewModel`/`RepositoryListViewModel` survive rotation as standard `ViewModel`s. Full typed back-stack/process-death restoration is explicitly out of this task's scope and already tracked as `handoff/claude-master-handoff.md`'s TASK-013; not re-raised here.
|
|
- Immutable snapshots (verified, no finding): `CodeBrowserUiState.Directory.entries` is wrapped in `Collections.unmodifiableList`.
|
|
- P3 (recorded, not fixed): `loadDirectory`/`loadFile` run entirely on `viewModelScope`'s default `Dispatchers.Main.immediate` after the suspending network call resumes — JSON parsing, Base64 decoding, directory sort/dedupe, and `SourceContentClassifier.classify()` (up to 1 MB/200,000 code points) all execute on the main thread. Nothing in this bound is large enough to reliably cause dropped frames, and `AGENTS.md`'s "never block main thread" rule targets network/disk I/O specifically (already off-thread via `OkHttpTransport`'s async callback), so this is not a P0-P2. Recommend wrapping the parse/classify/sort step in `withContext(Dispatchers.Default)` before TASK-016 (release hardening), since that task explicitly targets low-end Android 9 devices and large content. Closure condition: addressed in TASK-016 or a dedicated follow-up, whichever comes first.
|
|
- No P0/P1 found.
|
|
|
|
Device interaction remains deferred (no physical device available in this environment); this is an existing known limitation, not a new finding.
|