Skip to content

Conversation

cderv
Copy link
Collaborator

@cderv cderv commented Dec 22, 2023

If the input file is newer that the index file stored, then we should consider it outdated

this way is will be rewritten with newer content. cc @dragonstyle as we discussed

Context

Found while looking at #8016

The index file is read from local storage as a cache, but I notice that a complete unrelated version of my index.qmd was internally loaded while debugging. This is because we were finding the .qmd was newer but re-reading the existing (wrong) index file which leads the futher processing below to use an unrelated content to my real index.qmd

// check if we have an index that's still current vis-a-vis the
// last modified date of the source file
const index = readInputTargetIndexIfStillCurrent(projectDir, input);
if (!index) {
return {
missingReason: "stale",
};
}

export async function partitionedMarkdownForInput(
projectDir: string,
input: string,
) {
// first see if we can get the partioned markdown out of the index
const { index } = readInputTargetIndex(projectDir, input);
if (index) {
return index.markdown;
// otherwise fall back to calling the engine to do the partition
} else {
const inputPath = join(projectDir, input);
const engine = fileExecutionEngine(inputPath);
if (engine) {
return await engine.partitionedMarkdown(inputPath);
} else {
return undefined;
}
}
}

This PR makes sure we return undefined to make clear that no index file is found because stale. it will then be rewritten.

This is there since several month, I am surprised we were not bitten by that, but I am pretty sure something is not right. I believe this is fixed is the right one but unsure.

The new logic since performance fix (#5245) is more complex than the previous one which was

function readInputTargetIndexIfStillCurrent(projectDir: string, input: string) {
const inputFile = join(projectDir, input);
const indexFile = inputTargetIndexFile(projectDir, input);
if (existsSync(indexFile)) {
const inputMod = Deno.statSync(inputFile).mtime;
const indexMod = Deno.statSync(indexFile).mtime;
if (
inputMod && indexMod && (indexMod >= inputMod)
) {
try {
return JSON.parse(Deno.readTextFileSync(indexFile)) as InputTargetIndex;
} catch {
return undefined;
}
}
}
}

So hopefully I got it right

@cderv cderv added this to the v1.4 milestone Dec 22, 2023
@cderv cderv requested a review from cscheid December 22, 2023 20:10
@dragonstyle
Copy link
Collaborator

FWIW this looks correct to me.

@cscheid
Copy link
Collaborator

cscheid commented Jan 2, 2024

Nice catch! LGTM as well.

… consider it outdated

this way is will be rewritten with newer content
@cderv cderv force-pushed the fix/idx-file-outdated branch from bac63d8 to 0028633 Compare January 3, 2024 13:55
@cderv cderv merged commit 7ddc022 into main Jan 3, 2024
@cderv cderv deleted the fix/idx-file-outdated branch January 3, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants