feat(properties): warn when parenting exceeds depth cap of 5
Phase 5 polish. Soft cap — properties still save, but a console.warn fires so journalctl picks up a clear signal when a tree grows pathologically deep. Triggered on create + on parent reassignment via updateProperty. Justification for the warning: getDescendantIds runs an unbounded recursive CTE, and deep hierarchies are also painful to navigate in the existing list view. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -17,6 +17,28 @@ export interface PropertyCreateInput {
|
||||
notes?: string | null;
|
||||
}
|
||||
|
||||
// Soft cap. Properties past this depth still save, but get logged so a sudden
|
||||
// spike in journal entries draws attention to a tree growing pathologically.
|
||||
// Exists because getDescendantIds is an unbounded recursive CTE and deep
|
||||
// hierarchies are also painful to navigate in the UI.
|
||||
const MAX_RECOMMENDED_DEPTH = 5;
|
||||
|
||||
async function warnIfDeep(
|
||||
companyId: string,
|
||||
propertyId: string,
|
||||
candidateParentId: string
|
||||
): Promise<void> {
|
||||
const parentAncestors = await getAncestorIds(companyId, candidateParentId);
|
||||
const newChildDepth = parentAncestors.length + 1;
|
||||
if (newChildDepth > MAX_RECOMMENDED_DEPTH) {
|
||||
console.warn(
|
||||
`[properties] depth cap exceeded: parenting ${propertyId} under ${candidateParentId} ` +
|
||||
`places it at depth ${newChildDepth} (recommended max: ${MAX_RECOMMENDED_DEPTH}). ` +
|
||||
`Allowed but flagged.`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async function assertParentInCompany(companyId: string, parentId: string): Promise<void> {
|
||||
const [row] = await db
|
||||
.select({ id: properties.id })
|
||||
@@ -35,6 +57,7 @@ async function assertParentInCompany(companyId: string, parentId: string): Promi
|
||||
export async function createProperty(input: PropertyCreateInput): Promise<{ id: string }> {
|
||||
if (input.parentId) {
|
||||
await assertParentInCompany(input.companyId, input.parentId);
|
||||
await warnIfDeep(input.companyId, '<new>', input.parentId);
|
||||
}
|
||||
const values: NewProperty = {
|
||||
companyId: input.companyId,
|
||||
@@ -86,6 +109,7 @@ export async function updateProperty(
|
||||
if (patch.parentId === id) throw new Error('a property cannot be its own parent');
|
||||
await assertParentInCompany(companyId, patch.parentId);
|
||||
await assertNoCycle(companyId, id, patch.parentId);
|
||||
await warnIfDeep(companyId, id, patch.parentId);
|
||||
}
|
||||
await db
|
||||
.update(properties)
|
||||
|
||||
Reference in New Issue
Block a user