zenmemes

New post

Forte Activity Commits View — Step 4: Implementation Summary

Stage 1 - commits table populated from git log during read-model rebuild

  • Changes:

    • ReadModelBuilder::createSchema(): new commits table (id autoincrement PK, sha unique, author_name, author_email, committed_at, subject, file_count) plus commits_committed_at_idx for the pagination/sort access pattern Stage 2 will use; dropSchema() drops it alongside the other tables on rebuild.

    • New indexCommits(PDO $pdo): void, wired into rebuild()'s existing pipeline (measure('index_commits', ...)) alongside index_activity/etc. One git log --format=... --shortstat invocation (not one call per commit) produces every commit's metadata plus its file count in a single pass; fields are split on \x1f (unit separator - can't appear in a commit subject), and a commit with no --shortstat line (e.g. an empty/merge commit) defaults to file_count: 0 rather than being skipped.

  • Verification:

    • php -l - no syntax errors.

    • Ran php scripts/rebuild_read_model.php against the local fixture repo and queried the resulting commits table directly: 211 rows, exactly matching git -C state/local_repository log --oneline | wc -l.

    • Spot-checked the known large "Import repository archive" commit's row: subject and file_count (1955) both match the ground truth established earlier in this session (git show --stat on that sha).

    • Confirmed schema via PRAGMA table_info(commits) and sqlite_master - all 7 columns and the commits_committed_at_idx index present as designed.

  • Notes:

    • While verifying this stage, discovered the local fixture's content repository (state/local_repository) had accumulated 3,712 loose git objects with no packfile (from this long session's own testing), making the existing, unrelated per-file sourceCommitShaForPath() lookups (used by indexPosts()/indexThreadLabels()/etc., already present before this feature) take ~111ms each - across ~2,300 record files, that alone stretched a full rebuild from ~5s to ~144s. Ran git gc on the content repo (routine, non-destructive maintenance - packs loose objects, doesn't discard anything reachable) to restore normal performance; full rebuild is back to ~14s. This wasn't caused by indexCommits() and isn't part of this feature's scope, but was necessary to get a clean read while verifying it.

    • An initial read of the constructed git log command's printed output looked like the \x1f delimiter had been stripped by escapeshellarg() - turned out to be a false alarm: it's a non-printing control character, so it's simply invisible when echoed to a terminal, not actually absent from the string. The real rebuild's correctly-parsed 211 rows confirm the delimiter is intact and parsing works as designed.

Stage 2 - Sort-aware fetch/count for commits

  • Changes:

    • fetchCommits(string $sortColumn, string $sortDirection, ?array $afterCursor = null): array (Application.php) - mirrors fetchActivity()'s keyset-cursor pagination shape over the commits table; simpler than fetchActivity() since a commit has no "view" to filter by, so the only WHERE fragment is the cursor itself.

    • countCommitsTotal(): int - mirrors countActivityViewTotal()'s purpose for the left-pane folder count; every commit counts, no per-view filter.

    • resolveCommitSort()/commitSortSql()/commitSortValueFromItem() - mirror resolveActivitySort()/activitySortSql()/activitySortValueFromItem()'s shapes; only date is sortable for now per the Step 3 plan's own scope decision, kept as separate small resolvers (not folded into the activity versions) since the two views' valid column sets are unrelated.

    • Reuses ACTIVITY_ITEM_LIMIT as the page size rather than introducing a new constant for the same 100-row page size.

  • Verification:

    • php -l - no syntax errors.

    • Reflection-invoked fetchCommits() directly, paging through the full 211-row commits table with the real cursor: 100 + 100 + 11 items across 3 pages, has_more correctly true/true/false, zero duplicate ids, zero order violations, and the total unique ids collected (211) exactly matches countCommitsTotal().

  • Notes:

    • Same reflection-based verification approach used for fetchActivity() in the earlier sortable-columns feature, applied here for consistency.

Stage 3 - Wire "Commits" as a 6th filter

  • Changes:

    • normalizeActivityView()'s allowlist gains commits.

    • renderForteActivity(): fetches commits via fetchCommits()/countCommitsTotal() as a parallel row set ($commitItems), kept out of $itemsById/$items entirely rather than merged - commits don't share the activity-item shape. Adds a commits entry to both $viewPagination and $viewCounts; $selectedItemId is forced to '' when $selectedView === 'commits' (no $viewItemIds['commits'] to look up against - commit selection is Stage 4's job, not pre-selected here).

    • New templates/partials/paned_activity_commit_row.php - mirrors paned_activity_item_row.php's markup/CSS classes so it fits the existing list styling with no new CSS; maps short-sha/subject/committed_at onto the same Kind/Label/Date column positions. Uses data-paned-activity-id="commit-<sha>" (not the commit's own numeric id) since activity items and commits are both autoincrement ids from separate tables and could otherwise collide; a separate data-paned-activity-commit-sha attribute carries the raw sha for Stage 4.

    • paned_activity_item_list.php loops $commitItems after $items, rendering each via the new partial; visibility is $selectedView === 'commits' directly (commit items carry no view_* flags the way activity items do, since the rule is the same for every commit row). The existing Load More button loop already iterates $viewPagination generically, so it required no changes to pick up the new commits entry.

    • paned_activity_filter_list.php required no changes at all - it already loops $viewCounts generically, so the new commits entry just appears.

    • forte_activity.php threads $commitItems through to the list partial.

    • handleForteActivityPage() (/api/forte_activity_page): cursor decoding (identical shape for both) now happens once up front, then branches on view === 'commits' to fetchCommits() + the new row partial with an early return, leaving the existing activity-item path (rows + detail articles + shared manifest blocks) completely untouched below it. Commit pages always return detail_html: '' - Stage 4 fetches a commit's manifest on demand, not eagerly per row.

  • Verification:

    • php -l on all changed PHP/template files - no syntax errors.

    • Default page load: "Commits" filter appears with folder count 211 (matches countCommitsTotal()/Stage 1's row count exactly).

    • ?view=commits: page 1 has exactly 100 commit rows; a sample row (the known large commit) renders correctly - ff8f35ee366b (short sha) / Import repository archive repository.tar.gz (subject) / properly time-formatted date.

    • Paged through the full 211-commit set via /api/forte_activity_page?view=commits end to end: page 1 (100, has_more=true) -> page 2 (100, has_more=true) -> page 3 (11, has_more=false) - the known large commit correctly appears only on the last page (it's an old commit, so under default newest-first order it sorts near the end), confirming both the row count and the sort order are correct.

    • Confirmed commit rows carry hidden when the default view (all) is selected, and are absent from visibility (not merged into) the other 5 views' own row sets.

    • Regression: /api/forte_activity_page?view=all (existing Load More) still returns 100 rows correctly; classic /activity/, RSS (still exactly 100 items), and /backup/ all unaffected. Server log clean throughout.

  • Notes:

    • Selecting a commit row currently does nothing visible yet (no matching detail element exists) - this is the expected, temporary gap Stage 4 closes; Stage 3's own scope is the list/pagination wiring only, not detail rendering.

    • Per the Step 3 plan's own scope decision, clicking the Kind/Label sort headers while browsing Commits has no effect on commit order (resolveCommitSort() only recognizes date, falling back to it for anything else) - deliberate, not a bug.

Stage 4 - On-demand commit detail (full manifest), not pre-rendered eagerly

  • Changes:

    • New GET /api/forte_commit_detail?sha=<40-hex> (handleForteCommitDetail(), Application.php) reuses activityCommitManifest() (the same persistently-cached lookup the classic activity views already use) and renders the existing partials/activity_commit_manifest.php partial, returning just its HTML as JSON. A commit's manifest is deliberately not included in the Stage 3 row/page payload at all - some commits touch thousands of files, and only one is ever viewed at a time, so eagerly rendering every loaded row's manifest would recreate the exact page-bloat problem the earlier shared-manifest-block mechanism was built to work around. Malformed sha -> 400; well-formed but unknown sha -> 404.

    • paned_activity_reader.js: selectItem() now branches on whether the clicked/keyboard-navigated/URL-restored item id is a commit ("commit-<sha>" prefix). Commit selection goes through new showCommitDetail(sha), which builds a one-off <article class="paned-content-post" data-paned-activity-commit-detail="<sha>"> (kept out of the pre-rendered contentItems array entirely) with a head built immediately from the already-rendered row's own subject/date text (commitRowSubjectAndDate()) - no fetch needed for that part - and a body that shows "Loading…" until the manifest fetch resolves, then swaps in the returned HTML. A per-sha commitDetailCache object skips the fetch entirely for a commit already viewed this session. A late-arriving response for a commit the user has since navigated away from is discarded (checked by re-reading the article's current data-paned-activity-commit-detail attribute before writing), so rapid clicking across large commits can't let a slow response clobber a newer selection. resetContentPane() and the non-commit branch of selectItem() both hide the commit-detail article when leaving it, so it doesn't linger visible under an unrelated activity item or empty selection.

  • Verification:

    • php -l / node --check - no syntax errors.

    • GET /api/forte_commit_detail?sha=ff8f35ee366b7d0a7c20e88fe6d3ba01a06bdd4e (the known 1955-file commit): status: ok, returned HTML contains exactly 1955 <li> file entries - matches Stage 1's ground truth exactly.

    • GET /api/forte_commit_detail?sha=<a small, single-file commit>: status: ok, exactly 1 <li> entry.

    • Malformed sha (sha=notasha) -> 400. Well-formed but nonexistent sha (all-zero 40-hex) -> 404. Missing sha param -> 400.

    • GET /forte/activity/?view=commits (full page): 200, 100 commit rows present, "Commits" filter label renders, status bar shows "100 items" - unaffected by the new endpoint, since Stage 3 deliberately never eager-renders manifests.

    • Confirmed via curl that the served /assets/paned_activity_reader.js includes the new Stage 4 functions (showCommitDetail/commitDetailCache/the new endpoint URL), ruling out a stale-asset mismatch.

  • Notes:

    • No browser automation available in this environment for this stage (no chromium-cli, and the earlier CDP-attached-Chrome fallback used elsewhere in this session has known post-click navigation-wait limitations and process-leakage issues under repeated use) - verification here is HTTP/endpoint-level (exact response shape, file counts, status codes) plus careful code review of the client-side selection/caching/race-guard logic, consistent with how Stages 1-2 were verified (reflection/SQL, not a browser) before Stage 3 introduced markup that a page-level curl check could cover.

    • Selecting an activity item (not a commit) is unaffected: contentItems/showCommitManifestFor() (the existing shared-manifest-block mechanism for activity items, built during the earlier bug-fix work) are untouched by this stage - Stage 6 is where that mechanism gets removed, once Stage 5 gives activity items their own narrow relevant-files list instead of relying on it.

Stage 5 - Trim action detail views to relevant files only

  • Changes:

    • New Application::activityItemRelevantFiles(array $item): array builds an item's own narrow file list: its record, that record's detached signature (if any), and the signer's public key (if any) - plus, for identity_bootstrap items specifically, the paired identity record its own signature was signed with (resolved the same way activityCommitSignatureMetadata() resolves a signing key: signer identity -> OpenPGP fingerprint -> CanonicalPathResolver::identity(), not "any identity record the commit happens to contain").

    • Investigating real data surfaced a wrong assumption in an earlier draft of this method: a signing key is normally established once and reused across every later post it signs, so the key (and sometimes even the signature) commonly lives in a different, earlier commit than the record it signs - confirmed by inspecting several real commits (8e41631c5c80ff4...: record + signature only, no key). Filtering strictly against the item's own commit manifest (source_commit_files) would have silently dropped the public key from the relevant set in this - the common - case, contradicting the explicit requirement ("including the public key") the user asked for. Fixed by resolving the signature and public key independent of commit co-location: prefer the real manifest entry when the file happens to be part of the item's own commit (accurate git status), otherwise fall back to a standalone entry (standaloneRelevantFile()/standaloneSignatureRelevantFile()) built from activityCommitSignatureMetadata() and the item's own already-computed source_path_href/source_signature_href - correct regardless of which commit actually introduced the file. Only the item's own record (and identity_bootstrap's paired identity record) are still looked up against the item's own commit manifest first, since source_commit_sha is precisely "the commit that touched this path" for those two.

    • fetchActivity() computes relevant_files once per item (alongside the existing source_commit_files/source_signature_* fields it already builds), so it flows automatically into both the full-page render (renderForteActivity()) and the Load More/AJAX append path (handleForteActivityPage()) - both already built their item pools from fetchActivity().

    • activity_commit_manifest.php gains an optional heading parameter (default "Commit files", unchanged for the Commits view/Stage 4's endpoint) so paned_activity_detail_article.php can pass "Relevant files" instead - "Commit files (3)" on a 3-file subset of a commit would have read as wrong, not just imprecise.

    • paned_activity_detail_article.php: replaced the old $hasCommitManifest/shared-block-attribute logic with $relevantFiles = $item['relevant_files'], rendering activity_commit_manifest.php inline (no JS block-moving involved) when non-empty, falling back to the pre-existing source_metadata.php display when empty (mirrors the old fallback's trigger condition exactly, just switched from "commit manifest resolution failed" to "no source_path at all"). The article no longer carries data-paned-activity-commit-sha - Stage 3/4's shared-manifest-block JS (showCommitManifestFor()) still runs harmlessly on every selection (its sha lookup now always misses, so it just hides all shared blocks and moves nothing) until Stage 6 removes it outright.

  • Verification:

    • php -l on all changed PHP/template files - no syntax errors.

    • Reflection/HTTP tests against real fixture data, one case per code path: identity_bootstrap with its key established in the same commit (item 226) - 4 relevant files, in order (post record, identity record, detached signature, public key), all correctly linked. A normal signed post whose key was established in an earlier, different commit (item 1335, commit 8e41631c5c80ff4...) - 3 relevant files (record, signature, standalone public-key entry with status: current), with the signature's embedded "Public key:" link matching the standalone entry's own path exactly. An unsigned identity_bootstrap item sharing the 1955-file import commit (item 1, the same commit Stage 1 used as ground truth) - exactly 1 relevant file (its own record), not 1955 - this is the regression case the whole feature exists to fix. Two thread_label items with no signature - 1 relevant file each. In every case, heading read "Relevant files (N)" and the old source_metadata.php fallback did not render (relevant_files was non-empty for every item in this fixture dataset - no item here has an empty source_path, so the fallback branch itself is unexercised by live data, verified by code review instead).

    • Confirmed the resolved standalone public-key href for item 1335 actually resolves (GET /source/current/records/public-keys/openpgp-D07BA23778422C4D5C7A92B8A6C1CCEB2021496B.asc -> 200).

    • Regression: GET /api/forte_activity_page?view=all (Load More) - 100 articles, 1046 activity-commit-manifest sections total (~10 per item, i.e. no accidental duplication), 200 OK, no PHP warnings/fatals in the response body. Default /forte/activity/ page load - clean, no errors.

  • Notes:

    • paned_activity_reader.js was not touched in this stage - deliberately left as-is (see Changes above); Stage 6 is where showCommitManifestFor()/commitManifestBlocks and the PHP-side shared-block emission ($commitManifestsBySha in paned_activity_detail_pane.php/handleForteActivityPage()) get removed together, once this stage has proven the narrow relevant-files list is a complete replacement.

Stage 6 - Remove the shared-manifest-block machinery

  • Changes:

    • paned_activity_reader.js: removed showCommitManifestFor() and its call site in selectItem(), the commitManifestBlocks tracking array and its three assignments (init() and both places the AJAX append path refreshed it), and the shared-block dedup branch inside mergeAppendedDetailArticles() (a node's data-paned-activity-commit-manifest attribute never appears in server output any more, so that branch was already unreachable - removed rather than left as dead code). data-paned-activity-commit-sha on commit rows (Stage 3/4's own, distinct attribute, used by commitRowSubjectAndDate()) is unrelated and untouched.

    • paned_activity_detail_pane.php: removed the $commitManifestsBySha dedup-and-collect loop and the trailing foreach that rendered each unique commit's full manifest as a hidden block after the items loop - Stage 5 already made every item self-sufficient (relevant_files rendered inline), so nothing referenced these blocks any more.

    • Application::handleForteActivityPage() (the Load More/AJAX append path): removed the equivalent $commitManifestsBySha dedup loop that appended the same kind of hidden block onto $detailHtml per page.

  • Verification:

    • php -l / node --check on all changed files - no syntax errors.

    • GET /forte/activity/ (full page): 200, 0 occurrences of data-paned-activity-commit-manifest (down from Stage 3-5's non-zero count - the shared blocks are gone entirely, not just unused).

    • GET /forte/activity/?selected=226 (the 4-file identity_bootstrap case from Stage 5): still renders exactly 4 <li> entries under "Relevant files (4)" - selection/rendering is unaffected by removing the now-dead machinery around it.

    • GET /api/forte_activity_page?view=all (Load More): 200, 100 articles, 0 data-paned-activity-commit-manifest occurrences in the returned detail_html.

    • GET /forte/activity/?view=commits: 200, 100 commit rows still present - the Commits view (Stage 3) and GET /api/forte_commit_detail (Stage 4) are both untouched by this stage's removals, confirmed still working (large-commit manifest still returns correctly, 750KB/1955 files, matching Stage 4's own numbers exactly).

    • No PHP warnings/fatals/deprecation notices in any of the above responses.

  • Notes:

    • This closes out the Step 3 plan's full 6-stage scope. Every action's detail view now shows only its own relevant files (Stage 5) and the Commits view lets a reader browse and inspect actual git commits on demand (Stages 1-4), with no leftover shared-block machinery from the interim design.

Post-completion fixes (after "Approved Step 4" was requested)

  • Redundant public-key row. User feedback on item 1335's rendering: the signature's own entry already names and links the signer's public key ("Signer: ... / Public key: <link>"), and activityItemRelevantFiles() was also adding that same key as its own separate <li> - the two could never actually disagree (both resolved from the same signing-key lookup), so listing the file twice added nothing. Removed the standalone public-key entry from activityItemRelevantFiles(); a normal signed post now shows 2 relevant files (record, signature-with-embedded-key) instead of 3, and identity_bootstrap shows 3 (record, identity record, signature-with-embedded-key) instead of 4. Updated Step 2's "Core Requirements"/"Success Criteria" language to match (the key is part of the signature's own entry, not a fourth/third row). Verified via the same item 1335/226 cases used in Stage 5: <li> counts dropped by exactly one each, the embedded "Public key:" link is unaffected, and the Commits view's own full-manifest endpoint (where a public key genuinely is its own committed file, unrelated to any single action) is untouched - this only changed activityItemRelevantFiles(), not activityCommitManifest().

    • Regression-checked: Load More (/api/forte_activity_page?view=all, 100 articles), Commits view (?view=commits, 200 rows), and GET /api/forte_commit_detail (large commit, 750KB/1955 files unchanged) all still correct.

    • standaloneRelevantFile() is retained (still used for the identity_bootstrap identity-record fallback); only the public-key call site was removed.

  • Follow-up correction: the removal above was too broad. User clarified: when this action's own commit genuinely introduces the key (e.g. a fresh identity_bootstrap, which adds the key file alongside the record/signature it authenticates), that key is one of the files this action actually added and should still get its own row - the "don't double-list" reasoning only applies when the key is merely referenced from an earlier, unrelated commit (the common case for an ordinary signed post), where it isn't a file this action added at all. Restored the public-key row, but now conditioned on co-location: $files[] = $byPath[$publicKeyPath] only when the key is actually part of source_commit_files (i.e. isset($byPath[$publicKeyPath])) - never as a standalone/fallback entry for a key from a different commit. Re-verified: item 226 (identity_bootstrap, key co-located) is back to 4 relevant files; item 1335 (ordinary signed post, key from an earlier commit) stays at 2; item 1 (unsigned, giant import commit) stays at 1. Updated Step 2's wording again to reflect the conditional rule.

  • thread_label_add signing gap (user question, not a rendering bug). User asked why thread_label items show no signature at all. Traced to LocalWriteService::applyThreadTag() (the "Apply thread tag ... to ..." write path): it requires an author_identity_id but never calls the signature verifier or writes a .asc file, unlike the signed-post/signed-identity-bootstrap write paths in the same class. This is a genuine gap in the write service, not an Activity-view display bug - thread_label_add actions are, as currently implemented, unsigned by design/omission. No code changed for this - reported back to the user as a factual answer; whether to add signing to that write path is a separate decision outside this feature's scope.