Browse Source

fix: canonicalize URLs to one form per page, collapse existing duplicates

After the previous fix, the artdent scrape showed 438 distinct URLs in the
DB but only ~219 unique pages — every page was stored twice, once with and
once without a trailing slash. Sitemap (Yoast) emits the trailing-slash
form; LinkDiscovery had its own private normalizer that strips it; the two
forms were merged with exact string compare and both got saved.

Changes:

- New shared utility src/utils/url.ts::normalizeUrl(raw, base?) that
  resolves relative URLs, strips fragment + tracking params (utm_*,
  fbclid, gclid, ref), sort-orders query params, lowercases the host,
  and removes the trailing slash on the path (except root). Lifted from
  LinkDiscovery so every URL entry point uses the same rule.

- SitemapParser.parseSitemap normalises each <loc> before returning, and
  drops entries that can't be parsed as URLs.

- LinkDiscovery's private normaliser now delegates to the shared utility
  and just adds the same-origin constraint on top.

- WebScraper normalises custom URLs at intake and uses the canonical form
  for the dedup check against sitemap URLs.

- New SiteDatabase.cleanupTrailingSlashDuplicates(siteId), invoked at the
  end of each scrape (idempotent — no-op on a clean DB). For each
  (site_id, content_hash) group with multiple rows it keeps the
  canonical-URL row (or the earliest, if none are canonical) and DELETEs
  the rest. FK CASCADE clears site_embeddings; orphan Qdrant points are
  left for a future compact pass.
fszontagh 2 tháng trước cách đây
mục cha
commit
5bbab1e29f

+ 53 - 0
src/database/Database.ts

@@ -1,6 +1,7 @@
 import Database from 'better-sqlite3';
 import { v4 as uuidv4 } from 'uuid';
 import { logger } from '../utils/logger';
+import { normalizeUrl } from '../utils/url';
 import { ContentType } from '../types';
 import path from 'path';
 
@@ -1278,6 +1279,58 @@ export class SiteDatabase {
     }
   }
 
+  /**
+   * Remove rows whose URL is a non-canonical variant (e.g. trailing slash) of
+   * another row with the same content_hash for the same site. Keeps the row
+   * whose URL already equals normalizeUrl(url); if multiple already-canonical
+   * rows exist, the earliest one is kept.
+   *
+   * The FK CASCADE on site_embeddings clears per-content embedding records.
+   * Orphan Qdrant points (their IDs hash in the URL, so they differ from the
+   * canonical row's points) remain until a separate compact pass — they only
+   * waste storage and don't surface different content.
+   *
+   * Returns the URLs that were removed so callers can log them.
+   */
+  cleanupTrailingSlashDuplicates(siteId: string): string[] {
+    const rows = this.db.prepare(`
+      SELECT id, url, content_hash, created_at
+      FROM site_content
+      WHERE site_id = ?
+      ORDER BY content_hash, created_at ASC
+    `).all(siteId) as { id: string; url: string; content_hash: string; created_at: string }[];
+
+    const byHash = new Map<string, typeof rows>();
+    for (const r of rows) {
+      const arr = byHash.get(r.content_hash) ?? [];
+      arr.push(r);
+      byHash.set(r.content_hash, arr);
+    }
+
+    const idsToDelete: string[] = [];
+    const removedUrls: string[] = [];
+    for (const group of byHash.values()) {
+      if (group.length < 2) continue;
+      const canonical = group.filter(r => normalizeUrl(r.url) === r.url);
+      const keepers = canonical.length > 0 ? canonical : group;
+      const keepId = keepers[0].id;
+      for (const r of group) {
+        if (r.id !== keepId) {
+          idsToDelete.push(r.id);
+          removedUrls.push(r.url);
+        }
+      }
+    }
+
+    if (idsToDelete.length > 0) {
+      const placeholders = idsToDelete.map(() => '?').join(',');
+      this.db.prepare(`DELETE FROM site_content WHERE id IN (${placeholders})`).run(...idsToDelete);
+      const preview = removedUrls.slice(0, 5).join(', ');
+      logger.warn(`Removed ${idsToDelete.length} duplicate URL variants for site ${siteId}: ${preview}${removedUrls.length > 5 ? `, +${removedUrls.length - 5} more` : ''}`);
+    }
+    return removedUrls;
+  }
+
   /**
    * Clean up duplicate content entries by content_hash
    * When the same content appears in multiple categories, keep only the first occurrence

+ 7 - 35
src/scraper/LinkDiscovery.ts

@@ -1,6 +1,7 @@
 import axios from 'axios';
 import * as cheerio from 'cheerio';
 import { logger } from '../utils/logger';
+import { normalizeUrl } from '../utils/url';
 
 export interface DiscoveredUrl {
   url: string;
@@ -8,11 +9,6 @@ export interface DiscoveredUrl {
   foundOn: string;
 }
 
-const TRACKING_PARAMS = new Set([
-  'utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
-  'fbclid', 'gclid', 'ref',
-]);
-
 const SKIP_EXTENSIONS = new Set([
   '.js', '.css', '.png', '.jpg', '.jpeg', '.gif', '.svg', '.ico', '.pdf',
   '.zip', '.woff', '.woff2', '.ttf', '.eot', '.mp4', '.mp3', '.webp',
@@ -28,37 +24,13 @@ const SKIP_PATH_PATTERNS = [
 const PAGINATION_RE = /\/page\/\d+/;
 
 export class LinkDiscovery {
+  // Normalize relative-to-origin and then constrain to same-origin. Normalization
+  // rules live in src/utils/url.ts so sitemap and link-discovery URLs share a form.
   private normalizeUrl(raw: string, origin: string): string | null {
-    let parsed: URL;
-    try {
-      parsed = new URL(raw, origin);
-    } catch {
-      return null;
-    }
-
-    if (parsed.origin !== origin) {
-      return null;
-    }
-
-    // Strip fragment
-    parsed.hash = '';
-
-    // Remove tracking params
-    for (const param of TRACKING_PARAMS) {
-      parsed.searchParams.delete(param);
-    }
-
-    // Sort remaining params for consistency
-    parsed.searchParams.sort();
-
-    let href = parsed.href;
-
-    // Collapse trailing slashes but keep root /
-    if (href !== origin + '/' && href.endsWith('/')) {
-      href = href.replace(/\/+$/, '');
-    }
-
-    return href;
+    const normalized = normalizeUrl(raw, origin);
+    if (!normalized) return null;
+    if (new URL(normalized).origin !== origin) return null;
+    return normalized;
   }
 
   private shouldSkip(url: string): boolean {

+ 15 - 6
src/scraper/SitemapParser.ts

@@ -3,6 +3,7 @@ import { parseString } from 'xml2js';
 import { promisify } from 'util';
 import { SitemapUrl, SitePlatform, ContentType, ALL_CONTENT_TYPES } from '../types';
 import { logger } from '../utils/logger';
+import { normalizeUrl } from '../utils/url';
 import { getUserAgent } from '../version';
 
 const parseXml = promisify(parseString);
@@ -181,12 +182,20 @@ export class SitemapParser {
       }
       // Handle regular sitemap (contains URLs)
       else if (parsed.urlset && parsed.urlset.url) {
-        urls = parsed.urlset.url.map((urlEntry: any) => ({
-          loc: urlEntry.loc[0],
-          lastmod: urlEntry.lastmod?.[0],
-          changefreq: urlEntry.changefreq?.[0],
-          priority: urlEntry.priority?.[0]
-        }));
+        // Normalize loc URLs to the canonical form (no trailing slash, lowercase host,
+        // tracking params stripped) so they merge cleanly with LinkDiscovery output.
+        urls = parsed.urlset.url
+          .map((urlEntry: any) => {
+            const loc = normalizeUrl(urlEntry.loc[0]);
+            if (!loc) return null;
+            return {
+              loc,
+              lastmod: urlEntry.lastmod?.[0],
+              changefreq: urlEntry.changefreq?.[0],
+              priority: urlEntry.priority?.[0]
+            };
+          })
+          .filter((u: SitemapUrl | null): u is SitemapUrl => u !== null);
       }
 
       logger.info(`Parsed ${urls.length} URLs from sitemap`);

+ 14 - 6
src/scraper/WebScraper.ts

@@ -3,6 +3,7 @@ import { SitemapParser, resolveSitemapUrl } from './SitemapParser';
 import { LinkDiscovery } from './LinkDiscovery';
 import { ContentExtractor } from './ContentExtractor';
 import { logger } from '../utils/logger';
+import { normalizeUrl } from '../utils/url';
 import { SiteDatabase } from '../database/Database';
 import { WebhookManager } from '../webhooks/WebhookManager';
 import { QdrantEmbeddingWorkflow } from '../services/QdrantEmbeddingWorkflow';
@@ -145,21 +146,24 @@ export class WebScraper {
 
         for (const customUrl of customUrls) {
           const contentTypeKey = customUrl.content_type as ContentType;
+          const loc = normalizeUrl(customUrl.url);
+          if (!loc) {
+            logger.warn(`Custom URL is unparseable, skipping: ${customUrl.url}`);
+            continue;
+          }
 
-          // Check if this URL already exists in sitemap URLs
-          const alreadyExists = filteredUrls[contentTypeKey]?.some((u: any) => u.loc === customUrl.url);
+          const alreadyExists = filteredUrls[contentTypeKey]?.some((u: any) => u.loc === loc);
 
           if (!alreadyExists && customUrlsByType[contentTypeKey]) {
-            // Add custom URL in the same format as sitemap URLs
             customUrlsByType[contentTypeKey].push({
-              loc: customUrl.url,
+              loc,
               lastmod: undefined,
               changefreq: undefined,
               priority: undefined
             });
-            logger.info(`Added custom URL to ${contentTypeKey}: ${customUrl.url}`);
+            logger.info(`Added custom URL to ${contentTypeKey}: ${loc}`);
           } else {
-            logger.info(`Custom URL already in sitemap, skipping: ${customUrl.url}`);
+            logger.info(`Custom URL already in sitemap, skipping: ${loc}`);
           }
         }
 
@@ -264,6 +268,10 @@ export class WebScraper {
         if (deletedCount > 0) {
           logger.info(`Cleaned up ${deletedCount} disabled URLs that are no longer in sitemap`);
         }
+
+        // Collapse non-canonical URL variants (e.g. /araink + /araink/) into one
+        // row per content_hash. Cheap idempotent pass — on a clean DB it's a no-op.
+        this.db.cleanupTrailingSlashDuplicates(siteId);
       }
 
       const result: ScraperResult = {

+ 46 - 0
src/utils/url.ts

@@ -0,0 +1,46 @@
+/**
+ * Canonical URL form used everywhere we key by URL (DB rows, Qdrant point IDs,
+ * dedup sets). Sitemap URLs from Yoast/WordPress arrive with a trailing slash;
+ * link-discovery URLs arrive without one — without a shared normalizer, both
+ * forms get stored as separate rows.
+ *
+ * Rules (kept minimal because some sites do differentiate on query params and
+ * path case):
+ *   - resolve relative `raw` against `base` if provided
+ *   - strip the URL fragment
+ *   - drop common tracking params (utm_*, fbclid, gclid, ref)
+ *   - sort remaining query params for deterministic comparison
+ *   - lowercase the hostname (case-insensitive per RFC 3986); leave path case alone
+ *   - strip trailing slash on the path, except for the bare origin (`https://x/`)
+ *
+ * Returns null if the input can't be parsed into a URL.
+ */
+
+const TRACKING_PARAMS = new Set([
+  'utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
+  'fbclid', 'gclid', 'ref',
+]);
+
+export function normalizeUrl(raw: string, base?: string): string | null {
+  let parsed: URL;
+  try {
+    parsed = base ? new URL(raw, base) : new URL(raw);
+  } catch {
+    return null;
+  }
+
+  parsed.hash = '';
+  parsed.hostname = parsed.hostname.toLowerCase();
+
+  for (const param of TRACKING_PARAMS) {
+    parsed.searchParams.delete(param);
+  }
+  parsed.searchParams.sort();
+
+  let href = parsed.href;
+  const rootHref = `${parsed.origin}/`;
+  if (href !== rootHref && href.endsWith('/')) {
+    href = href.replace(/\/+$/, '');
+  }
+  return href;
+}