Repository navigation
Clarify full-text telemetry lifecycle and query-policy guidance - #800
kylebernhardy wants to merge 7 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the documentation across several files to clarify analytics buffer eviction priorities, configuration file deletion behaviors, and the introduction of new full-text search query limits (such as maximum query terms, fuzzy distance, and query clauses). The feedback suggests improving the clarity of these updates by explicitly explaining the rationale behind the analytics eviction priority and rephrasing the explanation of how omitted fuzzy distance parameters default to 1, which leads to query rejection when the maximum distance is configured to 0.
| `complete: false` flags lost final-handle counts. The next successful live observation starts a new `series` and restores complete coverage; use that observation as a new rate baseline. | ||
|
|
||
| Pending rows coalesce in a 128-row buffer per worker, dropping the oldest by first pending insertion on overflow; updating a pending row retains its position. While a group still has live handles, dropping a pending row preserves its cumulative counters for the next observation. After its last handle closes, losing its final pending row can lose those final totals; no later observation or `complete: false` row is guaranteed. `droppedRows` counts buffer evictions per worker and is repeated on its rows; do not sum it across indexes or roles. Workers and the receiving main thread enforce their current collection policy independently. During configuration transitions, the receiver can discard exported rows; those policy-filtered rows are not included in `droppedRows`. | ||
| Pending rows coalesce in a 128-row buffer per worker, on overflow evicting the least recently refreshed pending row from a still-live group before evicting a final row. Updating a pending row refreshes its position. While a group still has live handles, dropping a pending row preserves its cumulative counters for the next observation. After its last handle closes, losing its final pending row can lose those final totals; no later observation or `complete: false` row is guaranteed. `droppedRows` counts buffer evictions per worker and is repeated on its rows; do not sum it across indexes or roles. Graceful HTTP/job-worker exit drains pending rows and takes final observations after owner cleanup, including still-open handles. Forced termination can lose reports. Workers and the receiving main thread enforce their current collection policy independently. During configuration transitions, the receiver can discard exported rows; those policy-filtered rows are not included in `droppedRows`. |
There was a problem hiding this comment.
The phrase "evicting the least recently refreshed pending row from a still-live group before evicting a final row" describes the eviction priority, but it can be made clearer by explaining why still-live rows are prioritized for eviction. Since dropping a pending row from a still-live group preserves its cumulative counters for the next observation, whereas losing a final row can result in losing those totals permanently, explicitly stating this rationale improves clarity. To maintain technical accuracy, we should avoid merging these distinct prune points into a single clause.
Consider rephrasing to:
"Pending rows coalesce in a 128-row buffer per worker. On overflow, the least recently refreshed pending row from a still-live group is evicted first (preserving its cumulative counters for the next observation) before a final row is evicted."
References
- When simplifying or rewording technical documentation, avoid merging distinct conditions, event triggers, or prune points into a single clause if it compromises technical accuracy.
| - `maxResultsPerRequest` — Maximum retained offset plus limit. A request must also fit the separate raw-hit budget; default `10000`. | ||
| - `requestBudgetMilliseconds` — Execution budget across native batches, source checks and highlights, capped by the native execution limit; default `5000` ms. Reader acquisition and freshness waiting have separate bounds. | ||
| - `query.maxQueryTerms` — Maximum analyzed terms per text leaf; integer `1`–`1024`, default `64`. | ||
| - `query.maxFuzzyDistance` — Maximum fuzzy edit distance; integer `0`–`2`, default `1`. Distance `2` is preview and needs workload qualification; admission and deadlines are not hard process-memory bounds. With `0`, fuzzy and fuzzy-prefix conditions that omit `fuzzyDistance: 0` are rejected with HTTP 400; omitted distances remain `1`. |
There was a problem hiding this comment.
The phrasing "With 0, fuzzy and fuzzy-prefix conditions that omit fuzzyDistance: 0 are rejected with HTTP 400; omitted distances remain 1" is slightly confusing. It sounds like the conditions are rejected because they omit the value 0, whereas they are actually rejected because omitting the parameter causes it to default to 1, which exceeds the configured maximum of 0.
Clarifying this behavior makes the configuration option much easier to understand for operators.
Consider rephrasing to:
"If set to 0, fuzzy and fuzzy-prefix conditions that do not explicitly specify fuzzyDistance: 0 are rejected with HTTP 400, as omitted distances default to 1 (which exceeds the maximum of 0)."
References
- When documenting authorization or validation logic, avoid describing unreachable states that are rejected by validation APIs.
| | `matches_fuzzy` | Matches terms within one edit by default, including a transposition. | — | | ||
| | `matches_fuzzy_prefix` | Combines fuzzy matching (one edit by default) with a final-term prefix. | `surfaceTerms: true` | | ||
|
|
||
| <VersionBadge version="v5.4.0" /> On a `Table.search()` fuzzy or fuzzy-prefix condition, `fuzzyDistance` accepts an integer `0`–`2` and defaults to `1`. It must not exceed `storage.fullText.query.maxFuzzyDistance`; exceeding that maximum returns HTTP 400. With a maximum of `0`, set `fuzzyDistance: 0` explicitly. Distance `2` remains preview and requires workload qualification. |
There was a problem hiding this comment.
The phrasing "With a maximum of 0, set fuzzyDistance: 0 explicitly" can be clarified to explain the underlying default behavior. Since omitting the parameter defaults to 1, any query with an omitted distance will exceed the maximum of 0 and be rejected.
Consider rephrasing to:
"If the maximum is set to 0, fuzzyDistance: 0 must be set explicitly to avoid rejection, as omitted distances default to 1."
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-800 This preview will update automatically when you push new commits. |
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-800 This preview will update automatically when you push new commits. |
|
Heads-up: the v5 reference and the 5.4 release notes now live in the code repos (documentation#788, harper#3944, harper-pro#1237).
— Claude Opus 5.5 |
⊙ Problem
Operators need accurate lifecycle, configuration and interpretation guidance for Add opt-in native full-text analytics.
🤔 Why a follow-up?
The original Document opt-in full-text analytics and the 0.9 engine already merged. These corrections align its published guidance with the reviewed runtime refinements.
💡 Solution
Document collection defaults, policy transitions, cumulative counters and graceful final flushes. Align checkpoint compatibility and query-policy guidance with the runtime.
⚖️ Alternatives
Keep the existing analytics reference and configuration pages as the operator entry points. Avoid a separate telemetry API guide: internal consumers are implementation hooks, not a public subscription API.
🤔 Interpretation limits
Collection is optional, not a production overhead guarantee. Policy transitions and forced termination can lose observations; missing intervals are not zero activity.
🔧 Changes
Product and architecture tour
Configure, interpret and upgrade
Missing or invalid configuration keeps the last valid telemetry policy. Valid effective edits account for environment/in-process overlays. Buffer eviction drops the least recently refreshed live row before any final row. Graceful HTTP/job shutdown attempts final flushing.
Query guidance names fuzzyDistance, its default and over-maximum rejection. It separates full process restart from restart_service. Compatible 0.7/0.8 checkpoint reuse applies to nodes already running v5.4 prereleases.
Reference and release-note changes
reference/analytics/overview.md — explains retained policy, final commits, recent-refresh eviction and nullable metric metadata.
reference/configuration/operations.md — clarifies the full process restart required for captured query policy.
reference/configuration/options.md — defines query limits, explicit fuzzy-distance rejection and environment/override behavior.
reference/full-text-search/overview.md — limits compatible checkpoint reuse to nodes already running v5.4 prereleases.
reference/full-text-search/querying.md — names fuzzyDistance, its defaults and maximum, with v5.4 availability.
reference/resources/query-optimization.md — scopes checkpoint compatibility to the qualified prerelease migration route.
release-notes/v5-lincoln/5.4.md — summarizes query-policy controls, preview opt-in and restart requirements.
✅ Verification
Final documentation build processed 426 documents; formatting passes. No request/response example changed. Runtime regression evidence belongs to the companion core PR.
🤔 Review depth
Claude/domain reviewed
96831d13; the requested availability badge and narrow commit/null-metadata clarifications were then applied, formatted and built without another outside pass. Cumulative Claude/Composer coverage is established.Depends-on: HarperFast/harper#3946
Comment generated by kAIle (GPT-6)
Release target: v5.4 — companion analytics guidance
Complexity: medium
Related PRs: #777 overlaps, #778 overlaps, #788 overlaps
Deferred findings
b7fa803b170f80d5nit documentation —reference/configuration/options.md— docs say internal consumers can keep collection active; no core caller exists — filed Implement full-text generation lifecycle, rebuild, eviction, backup, and status harper#2510Review-Coverage: authored=codex; ran=cursor-composer,claude; adjudicated=domain; declined=gemini,cursor-grok,muse; rounds=9; full=3 @ ca9803c
Review-Attention: skim ~2m (decisions: checkpoint-compatibility-scope, missing-config-retention, fuzzy-distance-two-policy) @ ca9803c