Skip to content

Commit fa975d9

Browse files
committed
fix(slice-3-review): apply review findings
Must fix: - frontmatterRewrite: block-style topics with comments preserved - frontmatterRewrite: CRLF line endings preserved through rewrite - file_refs: store original_path alongside normalized path for case-sensitive filesystems (dead-refs on Linux) Should fix: - topics rename/delete: write topics.yaml before rewriting pages - health broken-links/broken-xwiki: filter archived source pages - topics list: page_count excludes archived (consistency with topics show) - topics.yaml header: document comment-stripping behavior - topics create: ensureFreshIndex before ad-hoc parent lookup - topics: hoist DB open out of iterating call sites (topicExists helper) - tag summary: only list newly-added topics Consider: - topicExists helper collapsing findTopic + isAdHocTopicInDb - use indexDbPath consistently in slice-3 commands - drop impossible error branch in runTopicsDescribe - fix misleading null/~ comment in topics/yaml.ts Tests: - block comments preserved across tag; CRLF round-trip - parameterized body-byte-preservation (LF/CRLF/mixed, block/flow) - archived wikilinks and xwiki filter from broken-links - topics list page_count excludes archived - topics create --parent ad-hoc (ensureFreshIndex path) - two-hop cycle via ad-hoc-promotion code path - --json shape snapshot for topics list/show and health - normalizePathPreservingCase unit tests
1 parent 6086fed commit fa975d9

15 files changed

Lines changed: 924 additions & 240 deletions

src/commands/health.ts

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -226,15 +226,25 @@ function findStale(
226226
* Only checks active pages — archived pages are allowed to reference
227227
* files that have since been deleted (that's often why they were
228228
* archived in the first place).
229+
*
230+
* We stat the `original_path` (author's casing) rather than the
231+
* lowercased `path` — on case-sensitive filesystems like Linux, stat
232+
* of a lowercased alias of `src/Dockerfile` returns ENOENT even
233+
* though the file exists. macOS and Windows are case-insensitive so
234+
* either form resolves there; using the original consistently means
235+
* the code behaves identically on every host.
229236
*/
230237
async function findDeadRefs(
231238
db: Database.Database,
232239
scope: HealthScope,
233240
repoRoot: string,
234241
): Promise<{ slug: string; path: string }[]> {
235242
const rows = db
236-
.prepare<[], { slug: string; path: string; is_dir: number }>(
237-
`SELECT p.slug, r.path, r.is_dir
243+
.prepare<
244+
[],
245+
{ slug: string; path: string; original_path: string; is_dir: number }
246+
>(
247+
`SELECT p.slug, r.path, r.original_path, r.is_dir
238248
FROM file_refs r
239249
JOIN pages p ON p.slug = r.page_slug
240250
WHERE p.archived_at IS NULL
@@ -244,19 +254,23 @@ async function findDeadRefs(
244254
const out: { slug: string; path: string }[] = [];
245255
for (const r of rows) {
246256
if (!inPageScope(scope, r.slug)) continue;
247-
// `r.path` is stored normalized & lowercased. On case-preserving
248-
// filesystems (macOS default) this still stats correctly because
249-
// HFS+/APFS are case-insensitive. On Linux the repo author is
250-
// expected to already be writing canonical casing.
251-
const abs = join(repoRoot, r.path);
257+
const abs = join(repoRoot, r.original_path);
252258
if (!existsSync(abs)) {
253-
out.push({ slug: r.slug, path: r.path });
259+
// Surface the author's casing in the report — matches what's in
260+
// the user's frontmatter/wikilink, which is what they'll search
261+
// for when fixing the miss.
262+
out.push({ slug: r.slug, path: r.original_path });
254263
}
255264
}
256265
return out;
257266
}
258267

259-
/** Wikilinks whose target slug has no row in `pages`. */
268+
/**
269+
* Wikilinks whose target slug has no row in `pages`. Every other
270+
* page-scoped check filters archived source pages out; this one and
271+
* `findBrokenXwiki` follow the same rule so the report doesn't flag
272+
* broken links from pages that have been retired.
273+
*/
260274
function findBrokenLinks(
261275
db: Database.Database,
262276
scope: HealthScope,
@@ -265,8 +279,9 @@ function findBrokenLinks(
265279
.prepare<[], { source_slug: string; target_slug: string }>(
266280
`SELECT w.source_slug, w.target_slug
267281
FROM wikilinks w
268-
LEFT JOIN pages p ON p.slug = w.target_slug
269-
WHERE p.slug IS NULL
282+
JOIN pages src ON src.slug = w.source_slug
283+
LEFT JOIN pages tgt ON tgt.slug = w.target_slug
284+
WHERE tgt.slug IS NULL AND src.archived_at IS NULL
270285
ORDER BY w.source_slug, w.target_slug`,
271286
)
272287
.all();
@@ -289,9 +304,14 @@ async function findBrokenXwiki(
289304
[],
290305
{ source_slug: string; target_wiki: string; target_slug: string }
291306
>(
292-
`SELECT source_slug, target_wiki, target_slug
293-
FROM cross_wiki_links
294-
ORDER BY source_slug, target_wiki, target_slug`,
307+
// Same archived-source filter as `findBrokenLinks`. Retired pages
308+
// shouldn't spam the report with links to wikis that may have
309+
// been intentionally retired too.
310+
`SELECT x.source_slug, x.target_wiki, x.target_slug
311+
FROM cross_wiki_links x
312+
JOIN pages src ON src.slug = x.source_slug
313+
WHERE src.archived_at IS NULL
314+
ORDER BY x.source_slug, x.target_wiki, x.target_slug`,
295315
)
296316
.all();
297317
const out: { source_slug: string; target_wiki: string; target_slug: string }[] = [];

src/commands/info.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,14 @@ function fetchInfo(db: Database.Database, slug: string): InfoRecord | null {
147147
.map((r) => r.topic_slug);
148148

149149
const refs = db
150-
.prepare<[string], { path: string; is_dir: number }>(
151-
"SELECT path, is_dir FROM file_refs WHERE page_slug = ? ORDER BY path",
150+
.prepare<[string], { original_path: string; is_dir: number }>(
151+
// Display the author's casing (`original_path`), not the
152+
// lowercased lookup form. The lowercased `path` column is the
153+
// query key for `--mentions`; it's not a user-facing string.
154+
"SELECT original_path, is_dir FROM file_refs WHERE page_slug = ? ORDER BY original_path",
152155
)
153156
.all(slug)
154-
.map((r) => ({ path: r.path, is_dir: r.is_dir === 1 }));
157+
.map((r) => ({ path: r.original_path, is_dir: r.is_dir === 1 }));
155158

156159
const linksOut = db
157160
.prepare<[string], { target_slug: string }>(

src/commands/tag.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
import { join } from "node:path";
2-
31
import { ensureFreshIndex, runIndexer } from "../indexer/index.js";
42
import { resolveWikiRoot } from "../indexer/resolveWiki.js";
53
import { openIndex } from "../indexer/schema.js";
64
import { toKebabCase } from "../slug.js";
75
import { rewritePageTopics } from "../topics/frontmatterRewrite.js";
8-
import { topicsYamlPath } from "../topics/paths.js";
6+
import { indexDbPath, topicsYamlPath } from "../topics/paths.js";
97
import {
108
ensureTopic,
119
loadTopicsFile,
@@ -92,8 +90,7 @@ export async function runTag(options: TagOptions): Promise<TagCommandOutput> {
9290
// `tag` — we just need to find each page's file; `ensureFreshIndex`
9391
// runs first so the common path is consistent.
9492
await ensureFreshIndex({ repoRoot });
95-
const dbPath = join(repoRoot, ".almanac", "index.db");
96-
const db = openIndex(dbPath);
93+
const db = openIndex(indexDbPath(repoRoot));
9794

9895
// Auto-create missing topics in topics.yaml.
9996
const yamlPath = topicsYamlPath(repoRoot);
@@ -134,9 +131,15 @@ export async function runTag(options: TagOptions): Promise<TagCommandOutput> {
134131
});
135132
if (result.changed) {
136133
taggedPages += 1;
137-
summary.push(`tagged ${page}: ${topics.join(", ")}`);
134+
// Only surface the NEWLY ADDED topics — not the full request.
135+
// Reporting every requested topic (including ones the page
136+
// already had) reads like false positives in commit diffs.
137+
const added = result.after.filter((t) => !result.before.includes(t));
138+
summary.push(`tagged ${page}: ${added.join(", ")}`);
138139
} else {
139-
summary.push(`no change ${page} (already tagged)`);
140+
summary.push(
141+
`no change ${page} (already tagged with ${topics.join(", ")})`,
142+
);
140143
}
141144
}
142145
} finally {
@@ -180,7 +183,7 @@ export async function runUntag(
180183
}
181184

182185
await ensureFreshIndex({ repoRoot });
183-
const db = openIndex(join(repoRoot, ".almanac", "index.db"));
186+
const db = openIndex(indexDbPath(repoRoot));
184187
let filePath: string;
185188
try {
186189
const row = db

0 commit comments

Comments
 (0)