refactor: Drop Last Updated column — idempotent 2-column memory indexes (ugde)#446
Open
sahil-noon wants to merge 5 commits into
Open
refactor: Drop Last Updated column — idempotent 2-column memory indexes (ugde)#446sahil-noon wants to merge 5 commits into
sahil-noon wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes fab memory-index branch-independent and idempotent by removing the domain/sub-domain index “Last Updated” column (a HEAD-relative git log projection) and updating the renderer, --check parsing/classification, tests, and documentation/migrations accordingly.
Changes:
- Drop the third “Last Updated” column from generated domain/sub-domain index tables and remove the now-dead date plumbing from the index path in Go.
- Update
--checkparsing/classification and refresh all related unit/golden tests for the 2-column format (while remaining tolerant of legacy 3-column rows). - Sweep specs/skills/memory docs, regenerate committed
docs/memory/**/index.mdfiles to 2-column format, and add a 2.6.6→2.7.0 migration + version bump.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/memory/_shared/index.md | Regenerated domain index to 2-column format and removed git-date note. |
| docs/memory/pipeline/index.md | Regenerated domain index to 2-column format and removed git-date note. |
| docs/memory/distribution/kit-architecture.md | Updated architecture prose to reflect dateless indexes + git pass serving log.md only. |
| docs/memory/distribution/migrations.md | Cataloged the new 2.6.6→2.7.0 migration and updated frontmatter description. |
| docs/memory/pipeline/execution-skills.md | Narrowed /git-pr 3a-bis rationale to log.md (index no longer date-driven) and updated hydrate text. |
| docs/memory/distribution/index.md | Regenerated domain index to 2-column format and removed git-date note. |
| docs/memory/memory-docs/hydrate-generate.md | Updated guidance to describe dateless index regeneration. |
| docs/memory/memory-docs/hydrate.md | Updated index ownership + maintenance prose to remove index date sourcing claims. |
| docs/memory/memory-docs/index.md | Regenerated domain index to 2-column format and removed git-date note. |
| docs/memory/memory-docs/templates.md | Updated templates/prose to describe content-only indexes and git pass usage for log.md. |
| docs/memory/pipeline/schemas.md | Updated schema prose to remove byPath/index-date projection and describe commitsByPath only. |
| docs/memory/runtime/index.md | Regenerated domain index to 2-column format and removed git-date note. |
| docs/specs/templates.md | Updated spec templates/examples and rationale to remove “Last Updated” column. |
| docs/specs/fkf.md | Updated FKF spec to define dateless domain/sub-domain indexes and adjust conformance wording. |
| docs/specs/skills/SPEC-docs-reorg-memory.md | Updated SPEC mirror to remove “Last Updated” references and clarify dateless index behavior. |
| docs/specs/skills/SPEC-git-pr.md | Updated SPEC mirror rationale for 3a-bis to be log.md-only. |
| fab/changes/260625-ugde-memory-index-drop-date-column/.history.jsonl | Added change audit trail. |
| fab/changes/260625-ugde-memory-index-drop-date-column/.status.yaml | Added pipeline status metadata for the change. |
| fab/changes/260625-ugde-memory-index-drop-date-column/intake.md | Added intake artifact documenting rationale, scope, and sweep class. |
| fab/changes/260625-ugde-memory-index-drop-date-column/plan.md | Added plan artifact with requirements/tasks/acceptance for the refactor + migration. |
| src/go/fab/cmd/fab/memory_index.go | Updated CLI help text to describe content-only indexes and remove index git-stamping references. |
| src/go/fab/internal/memoryindex/golden_test.go | Updated golden render expectations for 2-column domain indexes. |
| src/go/fab/internal/memoryindex/indexparse.go | Updated parser doc comment to match 2-column domain rows while ignoring trailing cells. |
| src/go/fab/internal/memoryindex/loss.go | Updated classifier doc comment to remove “refreshed date” from benign drift examples. |
| src/go/fab/internal/memoryindex/loss_test.go | Updated loss-classifier fixtures/tests to 2-column domain rows and added legacy-row tolerance test. |
| src/go/fab/internal/memoryindex/memoryindex.go | Dropped LastUpdated column rendering and removed index-only date plumbing; kept batched git pass for log.md. |
| src/go/fab/internal/memoryindex/memoryindex_test.go | Updated renderer/gather/git-log tests for dateless indexes and collapsed parseGitLog signature. |
| src/kit/VERSION | Bumped kit version to 2.7.0. |
| src/kit/migrations/2.6.6-to-2.7.0.md | Added migration guide to re-baseline existing repos to 2-column index format. |
| src/kit/reference/fkf.md | Updated normative reference mirror of FKF to match dateless index contract. |
| src/kit/skills/_cli-fab.md | Updated CLI reference docs for fab memory-index to remove index date behavior and narrow git-pass usage. |
| src/kit/skills/docs-hydrate-memory.md | Updated skill prose to remove index “Last Updated” guidance and git-date sourcing claims for indexes. |
| src/kit/skills/fab-continue.md | Updated skill prose to remove “Last Updated” references in regeneration guidance. |
| src/kit/skills/git-pr.md | Narrowed 3a-bis rationale to log.md and updated failure-mode description accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
+56
| ( cd "$probe" && fab memory-index >/dev/null 2>&1 ) | ||
| if grep -q 'Last Updated' "$probe/docs/memory/probe/index.md" 2>/dev/null; then | ||
| echo 'Aborted: this migration needs fab >= 2.7.0 (the two-column memory index). Upgrade the binary first: brew upgrade fab-kit.' | ||
| rm -rf "$probe" | ||
| exit 1 | ||
| fi |
Collaborator
Author
There was a problem hiding this comment.
Fixed — the probe now aborts when the generated index is missing too, not only when it contains a Last Updated header: added an explicit [ ! -f … ] guard before the grep -q, so a binary that errors and writes nothing aborts the migration instead of silently continuing. This now matches the prose note directly below the snippet. (27a3bde)
Comment on lines
46
to
50
| "and classifies drift by severity in the exit code: 0 = clean, 1 = benign " + | ||
| "drift (regen changes content but destroys nothing — all log.md / FKF " + | ||
| "drift (regen changes content but destroys nothing — an improved " + | ||
| "description:, all log.md / FKF " + | ||
| "frontmatter drift is benign; for log.md a benign FAIL means the committed " + | ||
| "log is missing a projected attributable (file-base, change-id) entry, or a " + |
Collaborator
Author
There was a problem hiding this comment.
Fixed — reworded the --check tier-1 help text to read "— e.g. an improved description:, or any log.md / FKF frontmatter drift", which removes the comma-splice run-on and backticks description: as inline code. (27a3bde)
… indexes (ugde) fab memory-index previously emitted a 3-column index table whose "Last Updated" cell was computed from git commit dates (HEAD/branch-relative). This caused content-identical indexes to differ across concurrent or un-rebased branches (e.g., loom PR #1846), breaking idempotency and creating spurious diffs. Fix: strip the "Last Updated" column entirely. Indexes are now 2-column (File | Description). The dated history stays in log.md, which is the correct home for per-entry timestamps. A 2.7.0 migration re-baselines existing repos. Sub-step 3a-bis in git-pr is retained but narrowed to log.md-only (index refresh is still a no-op when nothing drifted). Includes: Go implementation + tests, spec/memory updates, VERSION bump to 2.7.0, migration guide, and change artifacts for 260625-ugde.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rebase placed the change's own commit in history, so fab memory-index projects its per-folder log.md entries (the post-commit projection 3a-bis normally captures at ship). Generated with the 2.7.0 (2-column) binary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sahil87
force-pushed
the
260625-ugde-memory-index-drop-date-column
branch
from
a325a66 to
2e737e9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Meta
Pipeline: intake ✓ → apply ✓ → review ✓ → hydrate ✓ → ship → review-pr
Impact:
impl: +200/−141 (net +59)
tests: +103/−127 (net +-24)
total: +303/−268 (net +35) ← excludes
fab/,docs/Summary
fab memory-indexwas emitting a 3-column domain index table with a "Last Updated" cell computed fromgit logauthor dates (HEAD/branch-relative). Becausegit log -1 -- <file>is stable only for a fixed HEAD, regenerating the same index on a different branch (or after a rebase) projects a different date snapshot for unchanged files — causing content-identical indexes to diverge. This violated Constitution III (Idempotent Operations) and was the root cause of loom PR #1846's "lots of date-only changes" pattern.This PR fixes the problem by dropping the "Last Updated" column entirely: domain indexes are now 2-column (
| File | Description |), making them a pure function of file names and descriptions — genuinely branch-independent and idempotent. Dated, change-attributed history continues to live in the per-folder freeze-on-writelog.md, which is the correct home for per-entry timestamps. Bumps kit version to 2.7.0 with a migration guide that re-baselines existing repos.Changes
src/go/fab/internal/memoryindex/memoryindex.go):RenderDomaindrops the third column; removes now-dead date plumbing (FileEntry.LastUpdated,gitDates.byPath,(*gitDates).lookup,gitLastUpdated) while keeping the batchedgit logpass (still needed bylog.mdgeneration)--checkparser + classifier (indexparse.go,loss.go): updated to expect 2-column domain rows so tombstone/description/grouping detection continues to work after the format changesrc/go/fab/cmd/fab/memory_index.go): removed references to "stamping Last Updated from git" in theLongdescription and tier-1 exampleinternal/memoryindex/*_test.go): golden fixtures, render tests, and loss tests updated to 2-column format per Constitution VII (tests conform to spec)docs/specs/fkf.md,src/kit/reference/fkf.md,docs/specs/templates.md,docs/specs/skills/SPEC-*.md): updated to remove 3-column domain table references, reword git-date claims, and align with the new 2-column contractdocs/memory/**): 5 domainindex.mdfiles regenerated in 2-column form; 7 prose files updated to reflect the changesrc/kit/VERSION→ 2.7.0; new migrationsrc/kit/migrations/2.6.6-to-2.7.0.mdwith re-baseline instructionsDogfooding note
fab-kit's own
docs/memory/**/index.mdfiles are committed in 2-column form (built from this PR's source). The installed 2.6.6 binary would regenerate them as 3-column until 2.7.0 is released and installed — do not runfab memory-indexon this branch with the 2.6.6 binary, as it will corrupt the deliverable.